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

Implement Focused property #632

Merged
merged 1 commit into from
Nov 27, 2014
Merged

Conversation

sylvain-hamel
Copy link
Contributor

Issue: ChromiumWebBrowser.Focused is always false
Cause : cef does not implement focused correctly (seems related to https://code.google.com/p/chromiumembedded/issues/detail?id=563)
Fix : Manually implement Focused
How to test:

  • In the Windows Form sample, replace the Go Button event handler by MessageBox.Show(((Control)Browser).Focused.ToString()).
  • run
  • give the focus the the browser
  • click Go
  • result : false
  • apply fix
  • tests again
  • result : true
  • click the address bar (to remove focus)
  • click Go
  • result : false

I think this should fix #570. However if it does not, then this fix is still relevant to any code that relies on the Focused property. I did not test the WPF side of this at all.

Background: We've been running with this fix for a while now (before we started contributing to this project). I recalled this fix while reading #570 so here is the PR (I'll be able to remove this from our custom ChromiumWebBrowser derived class too).

@amaitland
Copy link
Member

I remembered there's a post in the google groups that I'd been meaning to do something about https://groups.google.com/forum/#!topic/cefsharp/myTd5OCjb5E

It relates to WinForms Focus. It appears there's two Focus methods that Cef exposes, one is OSR, the other regular. We've currently only implemented SendFocusEvent which is for OSR.

Thoughts on implementing SetFocus?

@amaitland amaitland added this to the 37.0.0 milestone Nov 22, 2014
@amaitland
Copy link
Member

I probably should have also made mention of CefFocusHandler which we only partly implement.

http://magpcss.org/ceforum/apidocs3/projects/%28default%29/CefFocusHandler.html

I think we can come up with a more elegant solution. Quite a while ago I looked at implementing CefFocusHandler it's here #341.

@amaitland
Copy link
Member

FYI #639 has been merged, which provides a CefFocusHandler wrapper. (Replaces #341)

@sylvain-hamel
Copy link
Contributor Author

@amaitland, IFocusHandler is useful to track focus events but does not resolve this issue. Using the FocusHandler does not make the Focused property work. It still returns false.

I tried to force the Focus() this way to see if it would set the Focused flag somehow in the control but it does not work.

public void OnGotFocus()
{
    ((Control)browser).BeginInvoke(new MethodInvoker(() => browser.Focus()));
}

So I still think this PR is necessary.

@amaitland
Copy link
Member

@sylvain-hamel I was thinking with OnGotFocus and OnTakeFocus we just need to store the bool and override Focused. When I get a moment I'll implement a rough solution.

@sylvain-hamel
Copy link
Contributor Author

I think the fix I proposed is better beause it fixes the problem in the core and does not force users to define a handler and to figure out how to handle the events. After all who wouldn't want Focus to retunr the right value?

@amaitland
Copy link
Member

Plan was to implement the default behavior in ChromiumWebBrowser, no need for users to do anything 😄

@jankurianski
Copy link
Member

Hi guys, I am currently working on a PR to implement something like what @amaitland is suggesting. (I feel a bit guilty about adding the FocusHandler but not addressing this core issue.)

I think it will turn out a bit cleaner because we won't have to rely on the Win32 APIs.

@amaitland
Copy link
Member

@jankurianski Awesome thanks! I'll hold off on making any changes 👍

@jankurianski
Copy link
Member

It turns out that it isn't possible to implement the Focused property via CEF's FocusHandler alone.

You can see what I tried here: https://github.com/numbersint/CefSharp/compare/fixes/winforms-focused-property

I implement the Focused property by setting the flag to true upon OnGotFocus, and false upon OnTakeFocus.

When I try the steps in the first post, the Focused property does get set to true, however it does not get set to false when I click in the address bar.

The reason for this is that FocusHandler has no method for focus loss. Here are how the methods work in common scenarios:

  • Clicking onto the browser
    1. OnGotFocus
  • Typing in an address and hitting Enter
    1. OnSetFocus
    2. OnGotFocus
  • Clicking off the browser, onto another control
    • (nothing)
  • Tab out of the browser
    1. OnTakeFocus

It would only be possible to implement the Focused property if IFocusHandler had a new method called OnLostFocus. But this would need to be an upstream CEF change.

For now, I think the solution of @sylvain-hamel here is the only way to go.

@jornh jornh modified the milestones: 37.0.0-pre01, 37.0.0 Nov 27, 2014
@amaitland
Copy link
Member

@jankurianski Appreciate you taking the time to have a look. I guess short term the fix we have is workable. Strange that focus events aren't called.

So let's go with this fix for now.


namespace CefSharp.Internals
{
public static class User32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this class into CefSharp.WinForms? it's unlikely to be used in any of the other projects.

@amaitland amaitland merged commit dd29410 into cefsharp:master Nov 27, 2014
@amaitland
Copy link
Member

Merged. I'm still hopeful that at some point we can get Cef to cooperate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking on Control using Winforms does not Set Focus
4 participants