1082510 - [e10s] Unable to select an item from a select list by click-drag-release

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mark Banner (:standard8)]

STR:

1) Open an E10s window, go to a page with select lists, e.g. https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox
2) Click-and-hold on the severity field
3) Move the mouse down

=> Note that the highlighted selection doesn't change.

4) Release the mouse

Actual results:

- Nothing changes, menu stays open

Expected results:

- The option hovered over should have been highlight, and on release the select list should have closed and selected the highlighted item.

Comment 2

[Jim Mathies [:jimm]]

Attachment: test case

Comment 3

[Jim Mathies [:jimm]]

Attachment: patch

I'm not sure about this change, but it does fix the problem: Remote content uses capture to keep receiving events [1] during a drag. Mouse move events over the e10s select popup initially route to the popup [2] near where we trap for select drop downs in non-e10s [3]. This routing then gets set back to the capture content here [4].

This patch updates the checks in [4]. Olli, thoughts?

[1] http://mxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#2784
[2] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7451
[3] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7483
[4] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7618

Comment 4

[Jim Mathies [:jimm]]

An alternative I guess would be to carry this popup check [1] down as a flag, and use it plus a remote capture check to override the !targetIsCrossDocDesc check.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7461

Comment 5

[Jim Mathies [:jimm]]

Comment on attachment 8564516 [details] [diff] [review]
patch

This patch caused a number of test failures.

Comment 6

[Jim Mathies [:jimm]]

Attachment: patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a33d0058ed28

Comment 7

[Jim Mathies [:jimm]]

Attachment: patch

correct patch this time.

Comment 8

[Jim Mathies [:jimm]]

Attachment: patch

This adds an additional condition before a retarget to captured content takes place - if the current target frame is a popup that is not a descendant of the captured content, redirect events to the popup.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=daf6bb36a813

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Odd. Why do we need that? What is the capturing content? Sounds like we should just make
the popup the capturing content, no?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8568014 [details] [diff] [review]
patch

Neil might have some comment about this.

Comment 11

[Jim Mathies [:jimm]]

(In reply to Olli Pettay [:smaug] from comment #9)
> Odd. Why do we need that? What is the capturing content?

Capturing content is the remote browser, which we set here - 

http://mxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#2766

With e10s, the select drop down isn't a descendent of the browser, so the ContentIsCrossDocDescendantOf check here fails -

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#7622

and we reset frame to capturingContent vs the popup frame.

> Sounds like we
> should just make
> the popup the capturing content, no?

Maybe, is that typical behavior here? I'll play around with this.

Comment 12

[Jim Mathies [:jimm]]

Attachment: patch

This is much simpler and works just as well.

Comment 13

[Jim Mathies [:jimm]]

Attachment: patch

- minor update

Comment 14

[Jim Mathies [:jimm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a1e3f673a10

Comment 15

[Jim Mathies [:jimm]]

review ping - nine days

Comment 16

[Neil Deakin]

Comment on attachment 8571668 [details] [diff] [review]
patch

This is probably ok as a workaround. The right fix would be to have the menulist capture the mouse, but I'm not sure if that would work as it isn't visible.

Comment 17

[Jim Mathies [:jimm]]

Attachment: patch r=Enn

Thanks!

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4f6461936cd2

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4f6461936cd2