Bug 27105

<rdar://problem/7043685>

Also, ctrl-tab to switch window tabs shows the same behavior.

Created attachment 39087 [details] Test case Updated test case to include window events and targets

Created attachment 39108 [details] Patch v1

Comment on attachment 39108 [details] Patch v1 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.

Created attachment 39228 [details] Fixes Eric's comments --- 6 files changed, 118 insertions(+), 4 deletions(-)

Created attachment 39229 [details] Fixes Eric's comments --- 5 files changed, 38 insertions(+), 42 deletions(-)

Sorry about the spam. My git+bugzilla-tool fu failed

Created attachment 39240 [details] All comments resolved.

Comment on attachment 39240 [details] All comments resolved. OK.

Comment on attachment 39240 [details] All comments resolved. 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

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. :)

I'll work on a new patch.

Created attachment 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 on attachment 39318 [details] Fixed iframe case Why does the blur count need to change? And why is the changed value correct?

(In reply to comment #15) > (From update of attachment 39318 [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?

(In reply to comment #16) > (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? Oh, so there is no functional change to the test, you're just correcting a typo?

Comment on attachment 39318 [details] Fixed iframe case Makes sense.

Comment on attachment 39318 [details] Fixed iframe case Clearing flags on attachment: 39318 Committed r48257: <http://trac.webkit.org/changeset/48257>

All reviewed patches have been landed. Closing bug.

This regressed clicking on autocompletion lists on windows: <rdar://problem/7391640>

This also caused a regression in which textfield callbacks are sent to editing clients: <https://bugs.webkit.org/show_bug.cgi?id=32904>

This led to a regression with onfocus handlers that call alert(). <rdar://problem/7880378>

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.

Maybe there is an obvious answer here - but how was the original change an improvement? Should we consider reverting this change?

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.

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