976673 - While mouseup event is being dispatched, event handlers cannot steal focus from inaccessible element if preceding mousedown event's preventDefault() is called

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Benoit Girard (:BenWa)]

Attachment: caret.gif

STR:
1) Open http://jsfiddle.net/fNPvf/2446/
2) Select the input box
3) Select each of the other boxes (HTML, CSS, JS)
4) You should now have several blinking carets.

Comment 1

[Benoit Girard (:BenWa)]

Reproducible on release FF 26.0

Comment 2

[Benoit Girard (:BenWa)]

And current release 27.0.1

Comment 3

[(no longer active)]

Sigh, I was hoping that this is a regression. :(

Comment 4

[Benoit Girard (:BenWa)]

Still worth trying it out on older revisions. This should fail early if it isn't.

Comment 5

[[:Aleksej]]

-g 2009-12-10-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64
-g 2009-12-11-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64 9af2a428dcb1
-b 2009-12-12-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64 d379a17cbf8f
-b 2009-12-13-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9af2a428dcb1&tochange=d379a17cbf8f

Comment 6

[(no longer active)]

(In reply to [:Aleksej] from comment #5)
> -g 2009-12-10-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64
> -g 2009-12-11-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64
> 9af2a428dcb1
> -b 2009-12-12-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64
> d379a17cbf8f
> -b 2009-12-13-03-mozilla-central-firefox-3.7a1pre.en-US.linux-x86_64
> 
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=9af2a428dcb1&tochange=d379a17cbf8f

Thanks, Aleksej!  It would be nice if someone could please bisect this range, none of those patches jump out at me as the culprit after a quick look.

Comment 7

[Alice0775 White]

In local build
Last Good: ade8ba415789
First Bad: a8f46161c891

Regressed by:a8f46161c891	Masayuki Nakano — Bug 125282 Webpage-JS can steal focus from URLbar / chrome r=enndeakin

Comment 8

[(no longer active)]

Huh, interesting!  Perhaps that bug changed things in a way that the editor is no longer notified when the field gets blurred out in order to hide the caret?  Masayuki, can you please take a look?  Thanks!

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Sure. It's odd. <input> element doesn't receive blur event...
http://jsfiddle.net/d_toybox/89JwW/

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Add tests

Hmm, this is very complicated.

First, the test page is under different domain. Therefore, other editors in parent document cannot steal focus from the test page.

The page prevents the default of mosuedown event. Therefore, ESM don't move focus to its parent document at mousedown event. However, the parent document tries to steal focus at mouseup event. Then, it fails.

We need to investigate other browsers' behavior before writing a patch.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This must be another case of bug 591890.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Add tests

Oops, forgot to add the new files.

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://jsfiddle.net/89JwW/3/
http://jsfiddle.net/89JwW/4/

Looks like that a call of preventDefault() of mosuedown doesn't allow to move focus.

However, from mouseup event handler, it allows to move focus both on Chrome and IE.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Add tests

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.2 Allow to steal focus by JS at handling mouseup event

This patch allows event handlers which run both dispatching mousedown and mouseup events to steal focus from any node.

This causes a lot of oragnes, though.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.3 Don't synthesize mouse events while another mouse event is being synthesized

This fixes most new oranges which is caused by unexpected NS_ASSERTION() calls.

The reason is, those tests synthesize another mouse events from mouse event handler. This means that input event is nested on the stack. This is unusual path and not testing actual behavior.

Therefore, this patch uses SimpleTest.executeSoon() for synthesizing another mouse or key events.

FYI: The hitting assertion is

NS_ASSERTION(!aDocument || !mMouseButtonEventHandlingDocument,
             "Some mouse button down events are nested?");

in nsFocusManager::SetMouseButtonHandlingDocument().

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.4 Allow nested mouse button event handling for modal dialog

Even if we land the patch part.3, there is some oranges.

That is caused by a mistake of the design of nsFocusManager::SetMouseButtonHandlingDocument(). It doesn't allow nested mouse button events. However, it occurs with modal dialog if it's opened from mouse button event handler. Therefore, we should allow to nested mouse button event.

This patch makes the method returns old document. And AutoHandlingUserInputStatePusher stores it and restores the document at destroying.

However, there is a concern. By this change, we will lose a way to detect nested input events which is typically caused by bad automated tests like fixed by part.3. But this is out of scope of this bug.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you for your review!

Enn: Don't you forget to review the part.1 patch? You did only 2 patches of 3 requests.

Comment 19

[Neil Deakin]

No I didn't forget. I needed more time to understand it.

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Neil Deakin from comment #19)
> No I didn't forget. I needed more time to understand it.

Okay, thank you.

Comment 21

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Neil Deakin from comment #19)
> No I didn't forget. I needed more time to understand it.

FYI: The test tests whether the parent document's script can steal focus from the iframe's document which is inaccessible from parent due to different domain.

The test case is, at mousedown event handler prevents it's default. This causes moving focus to the mousedowned document.
http://mxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#2698
http://mxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#2816

The message event is used for initializing and checking the result from the parent document to the iframe document.

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> This causes moving focus to the mousedowned document.

I meant this causes preventing to move focus to the mousedowned document.

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6abbb248f0b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/43d1907bd361
https://hg.mozilla.org/integration/mozilla-inbound/rev/79e075962df6
https://hg.mozilla.org/integration/mozilla-inbound/rev/8be6765c7d55

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6abbb248f0b2
https://hg.mozilla.org/mozilla-central/rev/43d1907bd361
https://hg.mozilla.org/mozilla-central/rev/79e075962df6
https://hg.mozilla.org/mozilla-central/rev/8be6765c7d55