Closed Bug 1417821 Opened 7 years ago Closed 7 years ago

`flushEventLoop` hangs forever if no `click` event is fired

Categories

(Remote Protocol :: Marionette, defect)

59 Branch
defect
Not set
normal

Tracking

(firefox58 unaffected, firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox58 --- unaffected
firefox59 --- fixed

People

(Reporter: barancev, Assigned: ato)

References

Details

Attachments

(5 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.89 Safari/537.36 Steps to reproduce: Using Selenium 3.7.0 and geckodriver 0.19 run this sample: driver.get("https://output.jsbin.com/balequc"); driver.findElement(By.id("over")).click(); Actual results: click operation hangs forever, reproduced on linux and windows. Works well on build https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=build%20windows&bugfiler&fromchange=b90598123a58&tochange=f67d247cf652&selectedJob=144680523 Fails on build https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=build%20windows%2064&bugfiler&selectedJob=145239835
Interesting testcase. And I can reproduce locally each time when using webdriverClick. So the only change which landed in the last 24h for that part is bug 1414329. Here a Marionette test: def test_click(self): self.marionette.navigate("https://output.jsbin.com/balequc") el = self.marionette.find_element(By.ID, "over") el.click() So clicking the div with the id `over` will not cause a `click` event at all. And given that there is no page load happening, `clickElement` waits forever for the promise to be resolved. Andreas, can you please have a look at this bug? Thanks.
Blocks: 1414329
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(ato)
Summary: 'click' operation hangs forever → `flushEventLoop` hangs forever if no `click` event is fired
This is a pretty interesting edge case because the DOM click event does not fire on <body> in Gecko, but does in Blink. I would have expected Blink’s behaviour to be correct but will have to verify this with someone with more DOM-foo than myself. annevk: When you have time, could you check what DOM events are expected to fire in https://sny.no/e/displaybubble? It registers mousedown, mouseup, and click on all the blue <div> boxes as well as <body>, and when you click the innermost box entitled “ONE”, its mousedown event sets its display style to "none". This apparently does not cause event to stop propagating (mouseup is still fired on THREE), but does in Gecko cause the click event to not fire on <body>.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Flags: needinfo?(annevk)
For the specific problem with Marionette, I’ve submitted a few patches that lets interaction.flushEventLoop time out (using our TimedPromise specialisation) when no click event is observed. This should fix the immediate problem at hand.
Flags: needinfo?(ato)
I don't think this is defined. File an issue with https://github.com/w3c/uievents? This probably relates to when and how often hit testing is performed while you get mouse input from the system. (I can trigger the body click listener in Fx by the way, so I don't think that's the problem.)
Flags: needinfo?(annevk)
Attachment #8931402 - Flags: review?(hskupin) → review+
Comment on attachment 8931403 [details] Bug 1417821 - Prevent missing click event from causing deadlock in interaction.flushEventLoop. https://reviewboard.mozilla.org/r/202544/#review208058 ::: commit-message-f8a53:4 (Diff revision 1) > +Bug 1417821 - Prevent missing click event from causing deadlock in interaction.flushEventLoop. r?whimboo > + > +If the DOM click event does not fire, which currently happens in > +this edge case where Firefox does not bubble the click event down It's bubbling up, not down. ::: testing/marionette/interaction.js:301 (Diff revision 1) > * Element that is expected to receive the click. > * > * @return {Promise} > * Promise is resolved once <var>el</var> has been clicked > - * (its <code>click</code> event fires) or the document is unloaded. > + * (its <code>click</code> event fires), the document is unloaded, > + * or a 1200 ms timeout is reached. This doesn't match the 500ms you are using in the code. So please update the documentation. ::: testing/web-platform/tests/webdriver/tests/element_click/bubbling.py:15 (Diff revision 1) > + > + > +def test_element_disappears_during_click(session): > + """ > + When an element in the event bubbling order disappears (its CSS > + displays tyle is set to "none") during a click, Gecko and Blink display style
Attachment #8931403 - Flags: review?(hskupin) → review+
Attachment #8931404 - Flags: review?(hskupin) → review+
Comment on attachment 8931405 [details] Bug 1417821 - Test that click returns after spinning event loop. https://reviewboard.mozilla.org/r/202548/#review208064
Attachment #8931405 - Flags: review?(hskupin) → review+
Comment on attachment 8931406 [details] Bug 1417821 - Add element click intercepted error to WPT WebDriver client. https://reviewboard.mozilla.org/r/202550/#review208066 ::: testing/web-platform/meta/MANIFEST.json:575974 (Diff revision 1) > "webdriver/tests/element_click/__init__.py": [ > "da39a3ee5e6b4b0d3255bfef95601890afd80709", > "support" > ], > "webdriver/tests/element_click/bubbling.py": [ > - "7c6777b805277d214cb4e5b27edaa898da7400c4", > + "1d798401c29616ced6c7a9fe21dea49837ecdd00", The update would have to be made for each individual commit to have a stable state, or?
Attachment #8931406 - Flags: review?(hskupin) → review+
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f46419561610 Use string representation of http_status. r=whimboo https://hg.mozilla.org/integration/autoland/rev/5f3a711342a6 Add element click intercepted error to WPT WebDriver client. r=whimboo https://hg.mozilla.org/integration/autoland/rev/12a1b7b8eb06 Prevent missing click event from causing deadlock in interaction.flushEventLoop. r=whimboo https://hg.mozilla.org/integration/autoland/rev/c1b476d02423 Test that click bubbles to parents. r=whimboo https://hg.mozilla.org/integration/autoland/rev/0662902bd2aa Test that click returns after spinning event loop. r=whimboo
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: