test timeout in events/test_focus_browserui.xul

RESOLVED FIXED in mozilla33

Status

()

RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: MarcoZ, Assigned: athena)

Tracking

(Blocks: 1 bug)

Trunk
mozilla33
x86_64
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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
(Reporter)

Comment 1

7 years ago
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?
(Reporter)

Comment 2

7 years ago
Created attachment 618301 [details] [diff] [review]
Disable file on OS X
(Reporter)

Comment 3

7 years ago
Disabled test on Mac: http://hg.mozilla.org/integration/mozilla-inbound/rev/247098e91bb5
Whiteboard: [leave open]

Comment 4

7 years ago
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
Assignee: nobody → marco.zehe
(Assignee)

Comment 6

4 years ago
Grabbing, metaKey should do the trick.
(Assignee)

Comment 7

4 years ago
Oh I missed that it was assigned!
(Reporter)

Comment 8

4 years ago
No worries, feel free to work on it!
Assignee: marco.zehe → nobody
(Assignee)

Comment 9

4 years ago
Created attachment 8449440 [details] [diff] [review]
Use metaKey to close the window on OSX
Attachment #8449440 - Flags: review?(marco.zehe)
(Reporter)

Comment 10

4 years ago
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.
Attachment #8449440 - Flags: review?(marco.zehe) → review+
(Assignee)

Comment 11

4 years ago
Created attachment 8449458 [details] [diff] [review]
close-window-mac-bug-746178.patch

I remember reading somewhere that try needs special privileges; not sure how to do it no!
Attachment #8449440 - Attachment is obsolete: true
(Reporter)

Comment 12

4 years ago
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! :)
(Assignee)

Comment 13

4 years ago
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.
Attachment #8449458 - Flags: checkin?
(Reporter)

Comment 14

4 years ago
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!
Attachment #8449458 - Flags: checkin? → checkin+
(Reporter)

Updated

4 years ago
Assignee: nobody → athena
Status: NEW → ASSIGNED
Whiteboard: [leave open]
(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.)
(Assignee)

Comment 16

4 years ago
Created attachment 8450078 [details] [diff] [review]
use accelKey instead of metaKey/ctrlKey

I do like how accellKey looks rather than the awkward if statement above.
Attachment #8450078 - Flags: review?(marco.zehe)
(Reporter)

Comment 17

4 years ago
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
Attachment #8450078 - Flags: review?(marco.zehe) → review+
https://hg.mozilla.org/mozilla-central/rev/2d229d0aa401
https://hg.mozilla.org/mozilla-central/rev/f8802f568bb4
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.