Last Comment Bug 786418 - browser_newtab_focus.js shouldn't fail when FKA is enabled on Mac
: browser_newtab_focus.js shouldn't fail when FKA is enabled on Mac
Status: RESOLVED FIXED
[qa-]
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 18
Assigned To: Andres Hernandez [:andreshm]
:
Mentors:
Depends on:
Blocks: 786084
  Show dependency treegraph
 
Reported: 2012-08-28 12:41 PDT by :Gavin Sharp [email: gavin@gavinsharp.com]
Modified: 2012-09-10 03:47 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
fixed
fixed


Attachments
Patch v1 (1.80 KB, patch)
2012-09-05 12:45 PDT, Andres Hernandez [:andreshm]
enndeakin: review+
Details | Diff | Splinter Review

Description :Gavin Sharp [email: gavin@gavinsharp.com] 2012-08-28 12:41:25 PDT
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) 2012-08-28 13:17:12 PDT
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 2012-08-28 13:29:48 PDT
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] 2012-09-05 12:45:25 PDT
Created attachment 658593 [details] [diff] [review]
Patch v1

Added pref to handle FKA case.
Comment 4 Neil Deakin 2012-09-06 07:20:44 PDT
Comment on attachment 658593 [details] [diff] [review]
Patch v1

># HG changeset patch
># User Andres Hernandez <andres@appcoast.com>
># 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) 2012-09-06 11:49:07 PDT
Fun fact: according to bug 786513, the 10.8 slaves do not actually have FKA enabled.
Comment 6 Andres Hernandez [:andreshm] 2012-09-06 16:48:29 PDT
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=78ff3d47ef53
Comment 7 Phil Ringnalda (:philor) 2012-09-06 17:50:54 PDT
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) 2012-09-06 20:25:30 PDT
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] 2012-09-07 08:46:27 PDT
https://hg.mozilla.org/mozilla-central/rev/f21fd828509b
Comment 11 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-09-10 03:47:52 PDT
(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?

Note You need to log in before you can comment on or make changes to this bug.