Bug 27105 - Blur and focus events are not fired when window is blurred and focused
: Blur and focus events are not fired when window is blurred and focused
Status: RESOLVED FIXED
: WebKit
HTML DOM
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
: HasReduction, InRadar, ReviewedForRadar
:
:
  Show dependency treegraph
 
Reported: 2009-07-08 17:34 PST by
Modified: 2010-12-07 20:52 PST (History)


Attachments
Test case (374 bytes, text/html)
2009-07-08 17:34 PST, Erik Arvidsson
no flags Details
Test case (1.38 KB, text/html)
2009-09-04 13:06 PST, Erik Arvidsson
no flags Details
Patch v1 (7.00 KB, patch)
2009-09-04 18:36 PST, Erik Arvidsson
no flags Review Patch | Details | Formatted Diff | Diff
Fixes Eric's comments (7.00 KB, patch)
2009-09-08 16:56 PST, Erik Arvidsson
arv: review-
Review Patch | Details | Formatted Diff | Diff
Fixes Eric's comments (5.62 KB, patch)
2009-09-08 16:56 PST, Erik Arvidsson
arv: review-
Review Patch | Details | Formatted Diff | Diff
All comments resolved. (7.34 KB, patch)
2009-09-08 18:37 PST, Erik Arvidsson
arv: review-
eric: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Fixed iframe case (9.65 KB, patch)
2009-09-09 18:34 PST, Erik Arvidsson
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-07-08 17:34:36 PST
Created an attachment (id=32492) [details]
Test case

Given an element that can be focused and has focus and blur event listeners. When the user tabs to the element or clicks on it the focus event is fired. Likewise, the blur event is fired when tabbing out or clicking elsewhere. The focus rectangle correctly reflects this. However, if i focus the element and switch to another window no focus nor blur events are fired.

The test prints the events being fired. Notice that no events are fired when the user switches between windows.
------- Comment #1 From 2009-07-08 19:59:19 PST -------
<rdar://problem/7043685>
------- Comment #2 From 2009-07-09 09:44:51 PST -------
Also, ctrl-tab to switch window tabs shows the same behavior.
------- Comment #3 From 2009-09-04 13:06:53 PST -------
Created an attachment (id=39087) [details]
Test case

Updated test case to include window events and targets
------- Comment #4 From 2009-09-04 18:36:54 PST -------
Created an attachment (id=39108) [details]
Patch v1
------- Comment #5 From 2009-09-08 14:49:59 PST -------
(From update of attachment 39108 [details])
Arv mentioned in-person that he wanted to update this.

changes which were discussed
shouldBeEqualToString (nit)
function for shorter js test (nit)
removal of "focus me" from bottom of test (nit)
comment in dispatchEventsOnWindowAndFocusedNode to document intended behavior.

In general this looks fine to me.
------- Comment #6 From 2009-09-08 16:56:43 PST -------
Created an attachment (id=39228) [details]
Fixes Eric's comments


---
 6 files changed, 118 insertions(+), 4 deletions(-)
------- Comment #7 From 2009-09-08 16:56:47 PST -------
Created an attachment (id=39229) [details]
Fixes Eric's comments


---
 5 files changed, 38 insertions(+), 42 deletions(-)
------- Comment #8 From 2009-09-08 17:01:51 PST -------
Sorry about the spam. My git+bugzilla-tool fu failed
------- Comment #9 From 2009-09-08 18:37:32 PST -------
Created an attachment (id=39240) [details]
All comments resolved.
------- Comment #10 From 2009-09-09 08:08:48 PST -------
(From update of attachment 39240 [details])
OK.
------- Comment #11 From 2009-09-09 08:19:30 PST -------
(From update of attachment 39240 [details])
Rejecting patch 39240 from commit-queue.

This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1
------- Comment #12 From 2009-09-09 09:37:48 PST -------
fast/events/tabindex-focus-blur-all.html -> failed
Seems we'll need new expected results.  Meaning unless someone would like to fix this manually on landing we'll need a new patch. :)
------- Comment #13 From 2009-09-09 11:13:43 PST -------
I'll work on a new patch.
------- Comment #14 From 2009-09-09 18:34:14 PST -------
Created an attachment (id=39318) [details]
Fixed iframe case

The last patch had real failures in it which was caught be the submit queue.

This patch handles iframes correctly and I extended the test case to cover that.
------- Comment #15 From 2009-09-09 18:42:40 PST -------
(From update of attachment 39318 [details])
Why does the blur count need to change?  And why is the changed value correct?
------- Comment #16 From 2009-09-10 08:54:51 PST -------
(In reply to comment #15)
> (From update of attachment 39318 [details] [details])
> Why does the blur count need to change?  And why is the changed value correct?

The blur count did not actually change. I just fixed the hard coded text to reflect the correct count. The expected count changed in a previous patch but the test was not updated back then.

-329 focus / 329 blur events dispatched, and should be 335 / 335 PASSED
+329 focus / 329 blur events dispatched, and should be 329 / 329 PASSED

I can take this file out of the patch if you prefer?
------- Comment #17 From 2009-09-10 09:38:19 PST -------
(In reply to comment #16)
> (In reply to comment #15)
> > (From update of attachment 39318 [details] [details] [details])
> > Why does the blur count need to change?  And why is the changed value correct?
> 
> The blur count did not actually change. I just fixed the hard coded text to
> reflect the correct count. The expected count changed in a previous patch but
> the test was not updated back then.
> 
> -329 focus / 329 blur events dispatched, and should be 335 / 335 PASSED
> +329 focus / 329 blur events dispatched, and should be 329 / 329 PASSED
> 
> I can take this file out of the patch if you prefer?

Oh, so there is no functional change to the test, you're just correcting a typo?
------- Comment #18 From 2009-09-10 09:39:07 PST -------
(From update of attachment 39318 [details])
Makes sense.
------- Comment #19 From 2009-09-10 10:03:59 PST -------
(From update of attachment 39318 [details])
Clearing flags on attachment: 39318

Committed r48257: <http://trac.webkit.org/changeset/48257>
------- Comment #20 From 2009-09-10 10:04:04 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #21 From 2009-11-23 13:31:39 PST -------
This regressed clicking on autocompletion lists on windows: <rdar://problem/7391640>
------- Comment #22 From 2009-12-23 10:39:17 PST -------
This also caused a regression in which textfield callbacks are sent to editing clients: <https://bugs.webkit.org/show_bug.cgi?id=32904>
------- Comment #23 From 2010-04-22 19:02:40 PST -------
This led to a regression with onfocus handlers that call alert(). <rdar://problem/7880378>
------- Comment #24 From 2010-04-22 19:32:27 PST -------
In general I think this is a bug in the web page but I agree that we need to defend against this case.

Here is what IE or Firefox do:

- IE does not dispatch the blur event
  Cons: The UI might not be correct because it did not receive the event

- Firefox dispatches the blur event but they do not restore focus so no second focus event is dispatched.
  Cons: The element does not get focus back so people using the keyboard will have to tab back to the element.

Other options include not showing the alert in a focus event which of course is also strange. 

Personally I think the best solution is to do what Chrome does and add a checkbox to the alert allowing the user to prevent more popups to be shown.
------- Comment #25 From 2010-04-22 23:54:45 PST -------
Maybe there is an obvious answer here - but how was the original change an improvement?  Should we consider reverting this change?
------- Comment #26 From 2010-04-23 09:29:13 PST -------
This fixes real world bugs [*].

Lets not revert a correct behavior due to a handful incorrect sites.

If this is causing iloops in high traffic sites a temporary revert on the release branch might be the correct answer until we solve the iloop alert problem. FWIW, Chrome has had this fix for a long time and has no reports of issues with this.  It might be due to it having a checkbox in the alerts to prevent all alert iloops.

[*] Almost all Google Apps rely on focus and blur events and one way the original bug manifests itself can be seen through the following steps:

1. Go to gmail
2. Focus the search field and type "is:"
3. This shows an auto complete popup
4. Focus another window

Expected result

The auto complete popup should be hidden.

On trunk this works but on Safari 4 the popup is not hidden when the element loses focus.
------- Comment #27 From 2010-06-11 15:01:40 PST -------
*** Bug 4814 has been marked as a duplicate of this bug. ***