Bug 37961 – Add a way for Widget to know about focus loss

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Stuart Morgan]

Currently, Widget has a setFocus() method to tell it that it has received focus, but no way of finding out that it has lost focus. This makes it impossible to correctly implemented focus for windowless plugins that don't have a corresponding native OS view--which Chromium on the Mac, at least, does not (and there are indications in the code that Mac Safari is, or at least once was) exploring the possibility of doing this.

[Stuart Morgan]

Created attachment 54008 [details]
Adds blur() to Widget

This adds blur() to Widget, along with an empty implementation on all platforms, and causes it to be called the same way setFocus() is currently called. Also adds the plumbing to connect blur() to the appropriate plugin method in Chromium (currently a no-op, but necessary for later non-WebKit Chromium changes).

No actual behavior changes should result from this patch. It will be up to platforms to start using this new method as they need it.

[WebKit Review Bot]

Attachment 54008 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/dom/Document.cpp:2873:  Declaration has space between type name and * in Widget *blurWidget  [whitespace/declaration] [3]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Stuart Morgan]

Created attachment 54069 [details]
Adds blur() to Widget

Fixes style issue (in both the new code and the code it was based on)

[Darin Fisher (:fishd, Google)]

Comment on attachment 54069 [details]
Adds blur() to Widget

WebCore/platform/Widget.h:155
 +      virtual void setFocus();
how about just passing a 'bool focused' to setFocus?

[Stuart Morgan]

The only reason is that it would be a much more invasive change. I would have to find and change the existing implementation of setFocus in every descendent of Widget in every port, which would touch a lot more files and actually change existing logic. The chance of a build break or regression would be much higher.

Since there didn't seem to be an obvious preference for bool-argument setters in other cases (e.g., Widget has  show() and hide() rather than setVisible(bool)), I opted for the simpler change. I can do it the other way if people feel strongly.

[Darin Fisher (:fishd, Google)]

It just struck me that 'setFocus' and 'blur' do not read like opposites.  Maybe the new method should be named setBlur?  That reads a bit funny though.

[Stuart Morgan]

Created attachment 55128 [details]
Adds boolean argument to setFocus

I can't come up with a better name, so I gave the setFocus(bool) approach a shot. The QT code unfortunately has a setFocus() call as well, but that was the only other one, so finding the right things to change wasn't as bad as I feared.

[WebKit Commit Bot]

Comment on attachment 55128 [details]
Adds boolean argument to setFocus

Clearing flags on attachment: 55128

Committed r58821: <http://trac.webkit.org/changeset/58821>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexander Mohr]

*** Bug 32836 has been marked as a duplicate of this bug. ***