746178 - test timeout in events/test_focus_browserui.xul

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Marco Zehe (:MarcoZ)]

Log from last passing test to end of test:
3978 INFO TEST-PASS | chrome://mochitests/content/a11y/accessible/events/test_focus_browserui.xul | test with ID = 'go back one page in history ' failed. There is unexpected focus event.
3979 INFO TEST-INFO | chrome://mochitests/content/a11y/accessible/events/test_focus_browserui.xul | Invoke the '[ 'document node', address: [object XrayWrapper [object HTMLDocument]] ] 't ctrl ' key' test { expected 'focus' event; unexpected 'focus' event;  }
3980 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_focus_browserui.xul | Test timed out.
3981 INFO TEST-END | chrome://mochitests/content/a11y/accessible/events/test_focus_browserui.xul | finished in 329999ms

Comment 1

[Marco Zehe (:MarcoZ)]

The problem here may very well be that we're sending the wrong keyboard shortcut to open a new tab on the Mac. It's not Ctrl+T as on Windows and Linux, but CMD+T. How do we generate the CMD key?

Comment 2

[Marco Zehe (:MarcoZ)]

Attachment: Disable file on OS X

Comment 3

[Marco Zehe (:MarcoZ)]

Disabled test on Mac: http://hg.mozilla.org/integration/mozilla-inbound/rev/247098e91bb5

Comment 4

[Ed Morley [:emorley]]

Sorry, push backed out for linux mochitest-other orange:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&onlyunstarred=1&rev=8cc13f37bc66
https://tbpl.mozilla.org/php/getParsedLog.php?id=11222399&tree=Mozilla-Inbound

{
4512 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/a11y/accessible/events/test_focus_listcontrols.xul | Error in test: proposed current item 'ml_tangerine' is already current
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/922510fc28aa

Comment 5

[Marco Zehe (:MarcoZ)]

Relanded with bustage fix: http://hg.mozilla.org/integration/mozilla-inbound/rev/175a8c81e3ff

Comment 6

[Athena]

Grabbing, metaKey should do the trick.

Comment 7

[Athena]

Oh I missed that it was assigned!

Comment 8

[Marco Zehe (:MarcoZ)]

No worries, feel free to work on it!

Comment 9

[Athena]

Attachment: Use metaKey to close the window on OSX

Comment 10

[Marco Zehe (:MarcoZ)]

Comment on attachment 8449440 [details] [diff] [review]
Use metaKey to close the window on OSX

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

Looks good! Please re-attach a patch with the nit fixed, but you don't need to re-request review. Do you know how to push to try? Would be good to get a test run on OS X before requesting checkin.

::: accessible/tests/mochitest/events/test_focus_browserui.xul
@@ +99,3 @@
>        // open new tab, focus moves to urlbar
> +      var synthKeyArgs;
> +      if ( MAC ) {

Nit: No spaces after opening and before closing parenthesis.

Comment 11

[Athena]

Attachment: close-window-mac-bug-746178.patch

I remember reading somewhere that try needs special privileges; not sure how to do it no!

Comment 12

[Marco Zehe (:MarcoZ)]

No problem at all, pushed to try on your behalf and letting Windows and OS X run the tests to make sure both variants are working. You can follow the progress here:
https://tbpl.mozilla.org/?tree=Try&rev=a2d569e6177e
However, Try should also push results to this bug when done. Let's keep our fingers crossed! :)

Comment 13

[Athena]

Comment on attachment 8449458 [details] [diff] [review]
close-window-mac-bug-746178.patch

Try hasn't left a comment, but based on that link it looks like all the tests passed, so I'm asking for a checkin.

Comment 14

[Marco Zehe (:MarcoZ)]

Comment on attachment 8449458 [details] [diff] [review]
close-window-mac-bug-746178.patch

Landed on Athena's behalf on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/2d229d0aa401

Thank you for the patch!

Comment 15

[[e-mail]]

(In reply to Marco Zehe from comment #1)
> The problem here may very well be that we're sending the wrong keyboard
> shortcut to open a new tab on the Mac. It's not Ctrl+T as on Windows and
> Linux, but CMD+T. How do we generate the CMD key?

(In reply to Athena from comment #6)
> Grabbing, metaKey should do the trick.

Actually it's even easier than that, accelKey should automatically switch from ctrl to cmd on the Mac. See _parseNativeModifiers in EventUtils.js line 611. (Note that getID doesn't understand either accel or cmd modifiers.)

Comment 16

[Athena]

Attachment: use accelKey instead of metaKey/ctrlKey

I do like how accellKey looks rather than the awkward if statement above.

Comment 17

[Marco Zehe (:MarcoZ)]

Comment on attachment 8450078 [details] [diff] [review]
use accelKey instead of metaKey/ctrlKey

Thanks for updating it! r=me and pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8802f568bb4

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2d229d0aa401
https://hg.mozilla.org/mozilla-central/rev/f8802f568bb4