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)

Product:
Remote Protocol
Component:
Marionette
Version:
59 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox58 unaffected, firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox58 --- unaffected
firefox59 --- fixed

People

(Reporter: barancev, Assigned: ato)

References

Details

Attachments

(5 files)

Bug 1417821 - Use string representation of http_status.
59 bytes, text/x-review-board-request
whimboo
: review+
Details
Bug 1417821 - Prevent missing click event from causing deadlock in interaction.flushEventLoop.
59 bytes, text/x-review-board-request
whimboo
: review+
Details
Bug 1417821 - Test that click bubbles to parents.
59 bytes, text/x-review-board-request
whimboo
: review+
Details
Bug 1417821 - Test that click returns after spinning event loop.
59 bytes, text/x-review-board-request
whimboo
: review+
Details
Bug 1417821 - Add element click intercepted error to WPT WebDriver client.
59 bytes, text/x-review-board-request
whimboo
: review+
Details 
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
status-firefox58: --- → unaffected
status-firefox59: --- → affected
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)
Comment on attachment 8931402 [details]
Bug 1417821 - Use string representation of http_status.

https://reviewboard.mozilla.org/r/202542/#review208054
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+
Comment on attachment 8931404 [details]
Bug 1417821 - Test that click bubbles to parents.

https://reviewboard.mozilla.org/r/202546/#review208062
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

Creator:
Created:
Updated:
Size: