1170166 - Add a capture flag to BrowserTestUtils.waitForEvent

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Neil Deakin]

Attachment: waitForEventCapture

Comment 1

[Steven MacLeod [:smacleod]]

Comment on attachment 8613524 [details] [diff] [review]
waitForEventCapture

Review of attachment 8613524 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a huge fan of adding the mandatory boolean argument here, but for the most part I'd r+ this. I'm going to pass this review off to Paolo though as he's more opinionated about the convention of BrowserTestUtils and I feel he'd be a better reviewer here.

Comment 2

[:Paolo Amadini]

Comment on attachment 8613524 [details] [diff] [review]
waitForEventCapture

Review of attachment 8613524 [details] [diff] [review]:
-----------------------------------------------------------------

Considering how rarely we ended up using the callback argument, I'd say we can add the optional "capture" argument before it. In fact the "focus" event can only be seen through a capturing listener and we don't support "focusin" yet, so we have a compelling use case.

::: layout/xul/test/browser_bug703210.js
@@ -19,5 @@
>      is(event.originalTarget.localName, "tooltip", "tooltip is showing");
>      return true;
>    });
>    yield BrowserTestUtils.synthesizeMouseAtCenter("#p1", { type: "mousemove" }, browser);
>    yield popupShownPromise;

For the record, I believe we could rewrite at least the first one (maybe the second too, if it doesn't rely on being synchronous) as:

let popupShownPromise = BrowserTestUtils.waitForEvent(document, "popupshown");
yield ...synthesizeMouseAtCenter...;
let event = yield popupShownPromise;
is(event.originalTarget.localName, "tooltip", "tooltip is showing");

::: testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ +242,5 @@
>     *        The element that should receive the event.
>     * @param {string} eventName
>     *        Name of the event to listen to.
> +   * @param {bool} capture
> +   *        True to use a capturing listener

@param {bool} capture [optional]

nit: dot at the end

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/94310347a60b

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/94310347a60b