1376857 - Port some mochitest events tests to browser tests

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Port events/test_{docload,focus_browserui}.xul to browser tests. r?yzen

Comment 2

[Eitan Isaacson [:eeejay]]

Attachment: Port events/test_focus_dialog.html to browser tests. r?yzen

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8881851 [details] [diff] [review]
Port events/test_focus_dialog.html to browser tests. r?yzen

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

cool

::: accessible/tests/browser/events/browser_test_focus_dialog.js
@@ +25,5 @@
> +  await BrowserTestUtils.closeWindow(newWin);
> +  testStates((await onFocus).accessible, STATE_FOCUSED);
> +
> +  onFocus = waitForEvent(EVENT_FOCUS, "body2");
> +  await ContentTask.spawn(browser, {}, async function() {

do we need async here?

@@ +37,5 @@
> +  testStates((await onFocus).accessible, STATE_FOCUSED);
> +
> +  let onShow = waitForEvent(EVENT_SHOW, "alertdialog");
> +  onFocus = waitForEvent(EVENT_FOCUS, "alertdialog");
> +  await ContentTask.spawn(browser, {}, async function() {

do we need async here?

Comment 4

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8881850 [details] [diff] [review]
Port events/test_{docload,focus_browserui}.xul to browser tests. r?yzen

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

thanks

::: accessible/tests/browser/e10s/browser_caching_description.js
@@ +143,5 @@
>      for (let { desc, waitFor, attrs, expected } of tests) {
>        info(desc);
>        let onUpdate;
>        if (waitFor) {
> +        onUpdate = waitForOrderedEvents(

I would rather update the waitFor fields in the declarative 'test' structure.

::: accessible/tests/browser/events.js
@@ +56,5 @@
> +  let acc = event.accessible;
> +  switch (typeof matchCriteria) {
> +    case "string":
> +      let id = getAccessibleDOMNodeID(acc);
> +      if (id == matchCriteria) {

nit: here and below ===

@@ +159,5 @@
>   * specified order.
>   * @param {Array} events        a list of events to wait (same format as
>   *                              waitForEvent arguments)
>   */
> +function waitForEvents(events, unexpected=[], ordered=false) {

nit: spaces around =

@@ +180,5 @@
> +    return results;
> +  });
> +}
> +
> +function waitForOrderedEvents(events, unexpected=[]) {

nit: spaces around = and jsdoc

::: accessible/tests/browser/events/browser_test_focus_browserui.js
@@ +36,5 @@
> +  EventUtils.synthesizeKey("w", { accelKey: true }, browser.ownerGlobal);
> +  evt = await onFocus;
> +  testStates(evt.accessible, STATE_FOCUSED);
> +
> +  info(evt);

is this still needed?

::: accessible/tests/browser/shared-head.js
@@ +203,5 @@
>      loadedScriptSet.add(browser);
>    }
>  }
>  
> +function snippetToURL(snippet, bodyAttrs={}) {

jsdoc,
nit: space around =

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Port events/test_focus_dialog.html to browser tests.

When a new window shows up, and you load a document, the resulting events are not determininstic in e10s. You may get a FOCUS event on the blank browser window, or on the loaded document. Opened a bug for this..

Comment 6

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8883645 [details] [diff] [review]
Port events/test_focus_dialog.html to browser tests.

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

looks good, thanks

Comment 7

[Eitan Isaacson [:eeejay]]

Attachment: Port events/test_focus_dialog.html to browser tests.

Just wait for a focus event for now, don't check the target. This should be fixed, and I opened bug 1377942 for that.

Comment 8

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8883679 [details] [diff] [review]
Port events/test_focus_dialog.html to browser tests.

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

looks good

::: accessible/tests/browser/events/browser_test_focus_dialog.js
@@ +17,5 @@
> +  let button = (await onFocus).accessible;
> +  testStates(button, STATE_FOCUSED);
> +
> +  // Bug 1377942 - The target of the focus event changes under different
> +  // circumstances. 

nit: whitespace

@@ +19,5 @@
> +
> +  // Bug 1377942 - The target of the focus event changes under different
> +  // circumstances. 
> +  // In e10s the focus event is the new window, in non-e10s it's the doc.
> +  onFocus = waitForEvent(EVENT_FOCUS, () => true);

Maybe we can do the something similar here (check for either doc or chrome window only)?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/27cbe1c0b7c5
Port events/test_{docload,focus_browserui}.xul to browser tests. r=yzen
https://hg.mozilla.org/integration/mozilla-inbound/rev/3497ed165c84
Port events/test_focus_dialog.html to browser tests. r=yzen

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/288575a7c30a
Port events/test_focus_dialog.html to browser tests: remove trailing whitespace to make eslint happy. r=eslint-fix

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/27cbe1c0b7c5
https://hg.mozilla.org/mozilla-central/rev/3497ed165c84
https://hg.mozilla.org/mozilla-central/rev/288575a7c30a