Skip to content

Conversation

@jankurianski
Copy link
Member

In our app we need to programmatically set focus to the web browser, but we found it wasn't possible (Control.Focus() doesn't do it). This implements SetFocus so we could call SetFocus(true), which does work (input focus moves to the browser).

Copy link
Member

Choose a reason for hiding this comment

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

Is it safe to new over the top of Focus?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a bit bad. I wasn't positive about this part of the change.

If someone has a ChromiumWebBrowser instance and calls Focus(), this implementation does get called and the browser gets focus. But if someone has a Control instance, it won't get called and the focus won't change.

So for example, if someone has a utility function like this that they pass a browser into as part of a list...

public void FocusLastControl(List<Control> controls)
{
    controls[controls.Count - 1].Focus();
}

... then it won't work (browser won't get focused).

I could just as easily leave this out so Focus() never works and doesn't give false hope to users, and they are forced to call SetFocus() which will always work. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I could just as easily leave this out so Focus() never works and doesn't give false hope to users, and they are forced to call SetFocus() which will always work. What do you think?

How about we leave that out for now, can discuss it further at a later date if required?

Copy link
Member

Choose a reason for hiding this comment

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

Happy to merge otherwise 👍

@amaitland amaitland added this to the 37.0.0 milestone Dec 10, 2014
@jankurianski
Copy link
Member Author

The new Focus() method is now gone. A good call, cheers @amaitland 👍

amaitland added a commit that referenced this pull request Dec 10, 2014
Implement CefBrowserHost::SetFocus for WinForms
@amaitland amaitland merged commit 8f7330f into cefsharp:master Dec 10, 2014
@amaitland
Copy link
Member

@jankurianski Thanks for the PR, much appreciated 👍

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.

2 participants