Closed Bug 1090602 Opened 10 years ago Closed 8 years ago

[e10s] <option> events do not bubble up through parent <select>

Categories

(Core :: DOM: Content Processes, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
e10s m9+ ---

People

(Reporter: KWierso, Assigned: mconley)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, dogfood, site-compat)

Attachments

(3 files, 5 obsolete files)

I think there's some weird timing issue when treestatus is running in an e10s window.

Steps:
0. Open an e10s window in Firefox Nightly.
1. Sign in to treestatus.
2. Select a (closed) tree from the list.
3. Change the 'new state' to another open and optionally clear the "reason" textbox.

Expected:
4. The Submit button changes from disabled and "No reason tags selected!" to enabled and "Submit".

Actual:
4. Submit button stays disabled and "No reason tags selected!"

The submit button only enables itself once you check one of the reason checkboxes. You can check one and uncheck it and the button will become and stay enabled.
This bug needs to be reported against Nightly-e10s, rather than treestatus. Even if treestatus is doing something weird, other commonly used third party sites may be doing the same, and e10s cannot afford to break them.

I tried to reproduce, but using e10s made my browser crash just trying to log into treestatus.m.o

Adjusting bug using the component/CC/meta bug specified for the file-a-bug link on https://wiki.mozilla.org/Electrolysis#Contributing

The javascript in question can be found here:
https://github.com/mozilla/treestatus/blob/master/treestatus/templates/tree.html#L19
Blocks: e10s
Component: TreeStatus → General
Product: Tree Management → Firefox
Summary: Changing a tree's status is weird when run in an e10s window. → [e10s] treestatus.mozilla.org UI doesn't update correctly when run in an e10s window
Version: --- → Trunk
tracking-e10s: --- → ?
Keywords: dogfood
Summary: [e10s] treestatus.mozilla.org UI doesn't update correctly when run in an e10s window → [e10s] treestatus.mozilla.org JS form validation breaks when run in an e10s window
Assignee: nobody → wmccloskey
(In reply to Ed Morley [:edmorley] from comment #1)
> I tried to reproduce, but using e10s made my browser crash just trying to
> log into treestatus.m.o
Ed, can you file a bug for this crash? Preferably with steps??
Flags: needinfo?(emorley)
If it's the crash I was experiencing on various sites before disabling all accessibility features, it was bug 1088148, which should now be fixed in either today's nightly or tomorrow's.
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #2)
> Ed, can you file a bug for this crash? Preferably with steps??

crash-stats.m.o had several already filed bugs, which is why I didn't file before, but have filed bug 1092216 in case it isn't a dupe.
Flags: needinfo?(emorley)
Assignee: wmccloskey → mconley
Flags: needinfo?(mconley)
I can reproduce this, and I suspect this is a focus event issue. I'll see if I can come up with a minimal test case.
Flags: needinfo?(mconley)
Attached file Manual test case (obsolete) —
Not a focus event issue - it's a click event issue. In non-e10s, a click event bubbles up from the <option> and gets handled by the click handler on the <select> element.

In e10s, no such event bubbles up.
Component: General → DOM: Content Processes
Product: Firefox → Core
Summary: [e10s] treestatus.mozilla.org JS form validation breaks when run in an e10s window → [e10s] <option> clicks do not bubble up through parent <select>
This seems to apply to all intrinsic events (click, mouseout, mousedown, keyup, etc).
Summary: [e10s] <option> clicks do not bubble up through parent <select> → [e10s] <option> events do not bubble up through parent <select>
Thank you for digging into this :-)
Depends on: 1053981
OS: Windows 8.1 → All
Hardware: x86_64 → All
So I've been looking at this bug for a few days, and here's what I've found out:

The nsListControlFrame listens to several events: keydown, keypress, mousedown, mouseup, and mousemove. If we can capture those events for the <option> elements on the parent side, send them down to the child, and replay them upon the corresponding <option> elements in the child, we get all of the battle-tested logic for the select element without too much fuss.

The problem is, however, that the nsListControlFrame, when it's in "dropdown" mode[1], manipulates and checks the state of that popup in various methods that are fired when those 5 events are dispatched to it.

So that means, in order for this plan to work, we'd need to trick nsListControlFrame into thinking that a popup is open, when in fact there is no popup open in its process. We do similar things for printing and the file input dialog, where we implement the interface on the child side, but then proxy method calls to the parent over IPC.

I guess that's what I'm proposing here, but I want a sanity check to make sure it makes sense.

ally, Enn - how feasible is it to create a new implementation of nsIComboboxControlFrame that implements some IPDL to communicate various things with the actual popup being opened on the parent side? I notice, for example, that there are some cases where the nsIComboboxControlFrame (mComboboxFrame) gets casted directly to an nsComboboxControlFrame, or QI'd to an nsIFrame, and we'd need to probably find ways around that (perhaps by adding methods to the nsIComboboxControlFrame that both the proxy and the original implementation can implement to hide the underlying implementation details).

So suppose we were to do that - what I see is something like this:

In the content process, we render a <select> element, and nsCSSFrameConstructor, seeing that it's in the content process, knows to instantiate and pass our proxy when calling SetComboboxFrame.

Once instantiated, the implementation sends the constructor message over IPC to the parent, which prepares the popup on the parent side. This would, unfortunately, have to occur in native code. The content process would need to serialize and communicate the children of the <select> up to the parent.

The parent then populates the popup, and listens for those 5 events on its cloned <option> and <optgroup> elements. On hearing them, those events are serialized and dispatched down to the child where they're replayed upon the associated <option> elements in the child. The nsListControlFrame then should, in theory, "behave normally", since our mocked our nsComboboxControlFrame is passing information to it which is consistent with the state in the parent.

This is all kinda sketchy, and based on only a few hours of me studying nsListControlFrame and friends, so it might be pretty half-baked. But I wanted to float it as an approach - especially because ally is working on bug 1053981 which is concerned with many of the same things[2].

Is there anything here I'm not considering?

[1]: The other mode is for multiple selections. In this mode, there is no popup/dropdown - we render the list in the page, and we do this successfully in content.
[2]: ally - I know I said we would likely stick with the JSMs, but if we end up needing to proxy out the nsIComboboxControlFrame into some kind of IPDL parent/child pair, it might make more sense to pack all of the logic of those JSMs into the parent/child instead. :/
Flags: needinfo?(enndeakin)
Flags: needinfo?(ally)
I don't know much about the select element code, so I don't I can give much of a helpful answer. Sorry.
Flags: needinfo?(enndeakin)
How about the other Neil then? :)
Flags: needinfo?(neil)
If it's a problem with getting click events to fire in the right place then smaug is probably your man. (But then again I don't know what part of this is being handled in the parent - I don't know if it's possible to get the child to paint the widget and receive events directly.)
Flags: needinfo?(neil)
(In reply to neil@parkwaycc.co.uk from comment #13)
> If it's a problem with getting click events to fire in the right place then
> smaug is probably your man. 

Alrighty, over to smaug then. :)

> (But then again I don't know what part of this
> is being handled in the parent - I don't know if it's possible to get the
> child to paint the widget and receive events directly.)

The way I'm picturing this, the rendering of the dropdown widget will be in the parent (my understanding is that it _has_ to be done in the parent), and any of those five events[1] that occur on that dropdown in the parent need to be dispatched to... I guess the nsListControlFrame in the child. And then the child can decide on how to react to those events by reading dropdown state synchronously over IPC.

It's all kinda theoretical though - I've not studied this code that much. I'm also very open to alternatives.
Flags: needinfo?(bugs)
Status: NEW → ASSIGNED
So how do we currently implement <select> (combobox) in E10s?
Flags: needinfo?(bugs)
Given the plan we hatched at the pdx workweek, is there still anything you need from me?
Flags: needinfo?(ally)
I don't think so - I think we're going to go with your idea basically, which was "try to special-case the content-process case in nsListControlFrame so that it doesn't have to care about the state of its dropdown to respond to events."
I've done quite a bit of digging at this, and I keep oscillating between "this won't be so bad", and "wow, this is harder than I thought". :/ I think there are likely bigger fish to fry right now, and maybe we can come back to this.

Nomming for retriage - my vote is m6 or m7.
Blocks: e10s-select
bumping to m8, we consider this a release blocker.
Assignee: mconley → gwright
Stealing because gw280 is on PTO.
Assignee: gwright → mconley
Bug 1090602 - <option> events do not bubble up through parent <select>. r=?
So I've posted my WIP / experimentation code here for someone to pick up in case they want to tackle this bug while I'm gone.

If somebody does that, they might want to read my bugnotes: https://www.evernote.com/l/AbIaeqtOzshJaogk1omBE_v_Dle4IbP8a4o

Specifically, the last few paragraphs show you what the current problems are.
Attached file Manual test case (obsolete) —
So the good news is that the original report in comment 0 no longer applies because TreeStatus has changed to use the onchange handler on the <select> instead of onclick (I'd point to that change in the code, except that it's all written in AngularJS which I'm not familiar with... suffice it to say that the event listener on the <select> is now listening for the change event).

Here's some interesting information:

The support for firing events on <option>'s is really spotty and inconsistent across browsers.

Here's a more general test case to test across other browsers.
Attachment #8523959 - Attachment is obsolete: true
Attached file Manual test case (obsolete) —
Some more refinements to the test case.
Attachment #8674443 - Attachment is obsolete: true
Attached file Manual test case (obsolete) —
Last bit of refinement.
Attachment #8674449 - Attachment is obsolete: true
Here are my findings for each of the browsers I tested:

Okay! Let’s do a quick study on <option> event support across browsers!


# Firefox (non-e10s - 44.0a1 (2015-10-15)):

* On hovering <option>’s with the mouse, mouseout, mousemove, mouseover are fired for each <option>
* On clicking an <option>, mousedown, mouseup, and click are fired for the clicked <option>
* keydown and keyup fired on the <select> when keying through each <option> in the dropdown
* keydown and keyup fired when hitting Enter to select an item


# Safari (Version 9.0 (10601.1.56.2)):

* When hovering <option>’s with the mouse, no events are fired.
* On click, the <select> gets mouseout, mouseup, mouseover, click, mouseout.
* Nothing is fired when keying through each <option> in the dropdown
* When pressing Enter on a selection, the <select> gets mouseout, mouseup, mouseover, click, keyup, and then mouseout


# Chrome (Version 46.0.2490.71 (64-bit))

* When hovering <option>’s, no events are fired on either the <option>’s or the <select>
* On clicking an <option>, no events are fired on either the <option>’s or the <select>
* Nothing is fired when keying through each <option> in the dropdown
* Nothing is fired when pressing Enter on a selection


# Internet Explorer (11.0.9600.18015)

* When hovering <option>’s, no events are fired on either the <option>’s or the <select>
* On clicking an <option>, the <select> fires a mousedown and a mouseup, and then the clicked <option> fires a <click> event.
* keydown and keyup are fired on the <select> when keying through each <option> in the dropdown
* keydown, keypress and keyup are fired on the <select> when pressing Enter on a selection


# Edge (20.10240.16384.0)

* When hovering <option>’s, no events are fired on either the <option>’s or the <select>
* On clicking an <option>, the <select> sees a mousedown, mouseup and then the clicked <option> fires a <click> event.
* keydown and keyup are fired on the <select> when keying through each <option> in the dropdown
* keydown, keypress and keyup are fired on the <select> when pressing Enter on a selection
Sent email to dev-platform suggesting that we converge with Chrome's behaviour here: https://groups.google.com/forum/#!topic/mozilla.dev.platform/9Li1-qBaM88
Attached file Manual test case (obsolete) —
Added click event on Sunday, and defaulting to inline <option>s.
Attachment #8674454 - Attachment is obsolete: true
Attached file Manual test case
Bah, fixup.
Attachment #8674974 - Attachment is obsolete: true
Submitted an issue to the WHATWG to hammer out a clearer spec on how (if ever) to dispatch events for <option> elements in dropdown <select>s.

https://github.com/whatwg/html/issues/263
Also cross-filed to the W3C: https://github.com/w3c/uievents/issues/55
This bug was marked M8 because we assumed it needed to be fixed in order to avoid breaking web compatibility.

I have now established via comment 27 and the issues I filed in comment 31 and comment 32 that the web compatibility requirements are undefined.

If we do nothing, we essentially match Chromium / Blink's behaviour (but break Firefox's original behaviour).

In any case, there doesn't seem to be a web compatibility risk here because of the fuzziness of the requirements. Re-requesting triage.
Hey Mike, I'm testing in Chrome 46 on Windows 10, and I get different results:

1. clicking an <option> fires "mouseup", "click", and "mouseout" events on the <select>
2. pressing Enter on a selection fires "mouseup", and "click" (!!) on the <select>; if the <select> was closed, pressing Enter opens the dropdown and fires "keydown", and "keypress" on the <select>

Also note that keying through options does fire key events if the dropdown is closed.
Flags: needinfo?(mconley)
(In reply to Šime Vidas from comment #34)
> Hey Mike, I'm testing in Chrome 46 on Windows 10, and I get different
> results:
> 
> 1. clicking an <option> fires "mouseup", "click", and "mouseout" events on
> the <select>
> 2. pressing Enter on a selection fires "mouseup", and "click" (!!) on the
> <select>; if the <select> was closed, pressing Enter opens the dropdown and
> fires "keydown", and "keypress" on the <select>
> 
> Also note that keying through options does fire key events if the dropdown
> is closed.

Wow - yes, I'm seeing the same thing on my Windows 7 machine running Chrome! I originally did all of my testing (minus the Edge and IE testing) from comment 27 on OS X.

That's very interesting - Chrome isn't consistent across operating systems.
Flags: needinfo?(mconley)
Chrome uses a native menu widget on Mac. (or something emulating it)
(In reply to Neil Deakin from comment #36)
> Chrome uses a native menu widget on Mac. (or something emulating it)

Right, which is probably what all browsers should be striving for - but the inconsistency in how it dispatches events is a bit surprising.
Right now if we don't patch e10s to match non-e10s Firefox capability, we're introducing more inconsistency (across versions) rather than less.
Alternatively, we could patch non-e10s to behave the same way as e10s. It's hard to know what the right move is here.
Flags: needinfo?(jgriffiths)
Mike, what are the web compat implications of this?
Flags: needinfo?(miket)
(Sorry for taking so long to reply here.)

OK, so based on the (really nice) summary Mike has given thus far, especially Comment #27, we can conclude that web browsers are generally insane.

My gut feeling is that there are likely no major compat implications beyond the current state of affairs if we don't fix this for e10s. *Some sites* may break (hence this bug report), but it seems pretty clear that if you're relying on x-browser click events to bubble up to select, you're gonna have a bad time.

We've also never supported this on Fennec, and I don't know about any mobile sites breaking -- otherwise I would have checked in the patch I wrote for Bug 715990 a few years back (we should WONTFIX that, I think, btw).

I would recommend we WONTFIX this, document the change, and teach people to listen for change events on <select>, rather than worry about <option>. That seems to be the general advice anyways for people asking similar questions:

http://stackoverflow.com/questions/1402227/click-event-on-select-option-element-in-chrome
http://stackoverflow.com/questions/7080213/jquery-click-event-not-working-on-option-element
http://stackoverflow.com/questions/5749597/jquery-select-option-click-handler

(Comment #38 is correct that we would be adding more entropy to the browser landscape by not fixing e10s to be consistent with non-e10s. Sorry about that.)
Flags: needinfo?(miket) → needinfo?(blassey.bugs)
Flags: needinfo?(blassey.bugs)
(In reply to Mike Taylor [:miketaylr] from comment #41)
...
> I would recommend we WONTFIX this, document the change, and teach people to
> listen for change events on <select>, rather than worry about <option>. That
> seems to be the general advice anyways for people asking similar questions:

BOOM. Done, and really appreciate your perspective :)
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: needinfo?(jgriffiths)
Resolution: --- → WONTFIX
Thank you, Kohei!
A number of improvements have been made to docs on MDN to cover and support this:

Cleanup and sample fixes on https://developer.mozilla.org/en-US/docs/Web/Events/change

These pages got notes mentioning the change:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/option
https://developer.mozilla.org/en-US/docs/Web/API/HTMLSelectElement

And I've created a page for covering any e10s related changes to Web content behavior, such as this one: https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Web_content_compatibility

Please have a look and if you see any problems, feel free to comment and set the keyword back to dev-doc-needed. Thanks!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: