Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WPF Moved capture into Opened/Closed events for Popup #1938

Merged
merged 2 commits into from
Mar 9, 2017

Conversation

taylorjonl
Copy link
Contributor

This commit is an attempt to fix the bug mentioned in this issue:

#1723

I modeled this change after the way the WPF ComboBox captures the mouse by looking at the code on referencesource.microsoft.com. It needs further testing to verify it doesn't break other stuff, hoping to get some more testing from one of my users tomorrow, wanted to start the conversation.

@amaitland
Copy link
Member

I modeled this change after the way the WPF ComboBox captures the mouse by looking at the code on referencesource.microsoft.com.

Do you have a link to the relevant file on the reference source?

@amaitland amaitland changed the title Moved capture into Opened/Closed events for Popup WPF Moved capture into Opened/Closed events for Popup Feb 14, 2017
@amaitland amaitland added the wpf WPF Implementation label Feb 14, 2017
@taylorjonl
Copy link
Contributor Author

Here is the link: https://referencesource.microsoft.com/#PresentationFramework/src/Framework/System/Windows/Controls/ComboBox.cs,26d4d26579e03147

The basics I learned from there are to capture when the ComboBox is activated and release capture when it is closed and the ComboBox is still capturing.

I think I need to work on this a bit more, I believe I broke use cases where a drag starts in the web browser and ends outside, I will update the pull request shortly.

@taylorjonl
Copy link
Contributor Author

I had a user testing this change for a few days and they didn't notice any issues. Not sure if there is any further testing you would like me to do or where to go from here?

@amaitland
Copy link
Member

I believe I broke use cases where a drag starts in the web browser and ends outside, I will update the pull request shortly.

@taylorjonl What was the end result with this?

@amaitland
Copy link
Member

Drop down lists appear to be broken, unable to select a different element with the mouse.

For testing I used CefSharp.Wpf.Example, open custom://cefsharp/TooltipTest.html and use one of the two drop down lists.

@taylorjonl
Copy link
Contributor Author

The changes I made to support what I thought was drag'n drop broke it. I pushed a new commit removing the code in the OnMouse[Up|Down] so it is back to the code I had my user testing. Drag'n drop still works with the code deleted so I am not really sure the purpose of the code. Before these changes combo boxes works because whenever the popup was entered it was capturing the mouse but the mouse and popup events were fighting a lot over capture which may be why the crash was occurring.

@amaitland
Copy link
Member

Code looks a lot cleaner now 👍 Overall I'm thinking it might be just easier to forward the mouse events directly, I'm not sure why Mouse.Capture was chosen as a solution. See c0e8e8e (I've merged it already as I'm just about to have lunch and this was a quick and dirty fix).

Feedback welcome.

@taylorjonl
Copy link
Contributor Author

I actually like that solution better, what are the chances you would do a new package against 55 with that change? The crash related to this capture is #2 on my list of crashes in my application and my managers are trying to chase %99.999 stability rate.

@amaitland
Copy link
Member

what are the chances you would do a new package against 55 with that change?

Is your company interested in sponsoring a 55.0.1 release? (55.0.0 was funded by user contributions see #1893 for more details).

@taylorjonl
Copy link
Contributor Author

This is my personal GitHub account, contact me at [email protected] and we can discuss this further.

@amaitland amaitland merged commit 5c70fb4 into cefsharp:master Mar 9, 2017
@amaitland amaitland added this to the 57.0.0 milestone Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wpf WPF Implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants