Closed Bug 1082510 Opened 10 years ago Closed 9 years ago

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

Categories

(Firefox :: General, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 39
Tracking Status
e10s m5+ ---
firefox39 --- fixed

People

(Reporter: standard8, Assigned: jimm)

References

Details

(Keywords: dogfood)

Attachments

(2 files, 6 obsolete files)

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.
Depends on: 1053981
Assignee: nobody → jmathies
Attached file test case
Attachment #8564187 - Attachment is patch: false
Attachment #8564187 - Attachment mime type: text/plain → text/html
Attached patch patch (obsolete) — Splinter Review
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
Attachment #8564516 - Flags: feedback?(bugs)
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 on attachment 8564516 [details] [diff] [review]
patch

This patch caused a number of test failures.
Attachment #8564516 - Flags: feedback?(bugs)
Attached patch patch (obsolete) — Splinter Review
correct patch this time.
Attachment #8567102 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
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
Attachment #8567104 - Attachment is obsolete: true
Attachment #8568014 - Flags: review?(bugs)
Odd. Why do we need that? What is the capturing content? Sounds like we should just make
the popup the capturing content, no?
Comment on attachment 8568014 [details] [diff] [review]
patch

Neil might have some comment about this.
Attachment #8568014 - Flags: review?(bugs) → review?(enndeakin)
(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.
Attached patch patch (obsolete) — Splinter Review
This is much simpler and works just as well.
Attachment #8568014 - Attachment is obsolete: true
Attachment #8568014 - Flags: review?(enndeakin)
Attachment #8571661 - Flags: review?(enndeakin)
Attached patch patch (obsolete) — Splinter Review
- minor update
Attachment #8571661 - Attachment is obsolete: true
Attachment #8571661 - Flags: review?(enndeakin)
Attachment #8571668 - Flags: review?(enndeakin)
review ping - nine days
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.
Attachment #8571668 - Flags: review?(enndeakin) → review+
Attached patch patch r=EnnSplinter Review
Thanks!
Attachment #8571668 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4f6461936cd2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Depends on: 1146001
No longer depends on: 1146001
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: