786418 - browser_newtab_focus.js shouldn't fail when FKA is enabled on Mac

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

This test fails when FKA is on, because it assumes that links won't be focused. It would be ideal to have the test detect the state of the pref somehow (either heuristically - I think other tests do this - or via some exposed API), and adjust its expectations accordingly.

Philor brought this up on IRC because apparently FKA is on by default in 10.8 (or was enabled on our reference images somehow). This test also always fails when I run it locally, and it would be nice if it didn't.

Comment 1

[Phil Ringnalda (:philor)]

If the new tab page is XUL, http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/browser/browser_list.js#611 implies that just setting accessibility.tabfocus_applies_to_xul to false should make the FKA setting not matter.

Comment 2

[Neil Deakin]

The test is using links though. If they are <html:a> elements, then the 'accessibility.tabfocus' pref should be used instead.

Comment 3

[Andres Hernandez [:andreshm]]

Attachment: Patch v1

Added pref to handle FKA case.

Comment 4

[Neil Deakin]

Comment on attachment 658593 [details] [diff] [review]
Patch v1

># HG changeset patch
># User Andres Hernandez <[e-mail]>
># Date 1346874221 21600
># Node ID 6ebab4e8d9d9202e828020c8b0b5735f3146b7dd
># Parent  938dacff2d5caff3d97e3517850ca6f3c3f6b017
>Bug 786418 - browser_newtab_focus.js shouldn't fail when FKA is enabled on Mac r=gavin
>
>diff --git a/browser/base/content/test/newtab/browser_newtab_focus.js b/browser/base/content/test/newtab/browser_newtab_focus.js
>--- a/browser/base/content/test/newtab/browser_newtab_focus.js
>+++ b/browser/base/content/test/newtab/browser_newtab_focus.js
>@@ -1,37 +1,38 @@
> /* Any copyright is dedicated to the Public Domain.
>    http://creativecommons.org/publicdomain/zero/1.0/ */
> 
> /*
>  * These tests make sure that focusing the 'New Tage Page' works as expected.
>  */
> function runTests() {
>+  // Handle the OSX full keyboard access setting
>+  Services.prefs.setIntPref("accessibility.tabfocus", 7);
>+
>   // Focus count in new tab page.
>   // 28 = 9 * 3 + 1 = 9 sites and 1 toggle button, each site has a link, a pin
>   // and a remove button.
>-  let FOCUS_COUNT = 28; 
>-  if ("nsILocalFileMac" in Ci) {
>-    // 19 = Mac doesn't focus links, so 9 focus targets less than Windows/Linux.
>-    FOCUS_COUNT = 19;
>-  }
>+  let FOCUS_COUNT = 28;
> 
>   // Create a new tab page.
>   yield setLinks("0,1,2,3,4,5,6,7,8");
>   setPinnedLinks("");
> 
>   yield addNewTabPageTab();
>   gURLBar.focus();
> 
>   // Count the focus with the enabled page.
>   yield countFocus(FOCUS_COUNT);
> 
>   // Disable page and count the focus with the disabled page.
>   NewTabUtils.allPages.enabled = false;
>   yield countFocus(1);
>+
>+  Services.prefs.clearUserPref("accessibility.tabfocus");
> }
> 
> /**
>  * Focus the urlbar and count how many focus stops to return again to the urlbar.
>  */
> function countFocus(aExpectedCount) {
>   let focusCount = 0;
>   let contentDoc = getContentDocument();

Comment 5

[Phil Ringnalda (:philor)]

Fun fact: according to bug 786513, the 10.8 slaves do not actually have FKA enabled.

Comment 6

[Andres Hernandez [:andreshm]]

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=78ff3d47ef53

Comment 7

[Phil Ringnalda (:philor)]

Well, https://tbpl.mozilla.org/?tree=Try&rev=78ff3d47ef53&noignore=1 since you want to see the still-hidden 10.8.

Comment 8

[Phil Ringnalda (:philor)]

And that's green like a green thing with green stuff on it, sitting on a green thing that's on another green thing \o/

https://hg.mozilla.org/integration/mozilla-inbound/rev/f21fd828509b

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f21fd828509b

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9cfb3a0c64d5
https://hg.mozilla.org/releases/mozilla-beta/rev/8aa8d545400a

Comment 11

[:Gavin Sharp [email: [e-mail]]]

(In reply to Phil Ringnalda (:philor) from comment #5)
> Fun fact: according to bug 786513, the 10.8 slaves do not actually have FKA
> enabled.

Maybe our FKA-enabled detection is broken on 10.8?