Last Comment Bug 491624 - [SeaMonkey, Linux] mochitest-browser-chrome: intermittent ".../suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got [object HTMLInputElement], expected [object XULElement]"
: [SeaMonkey, Linux] mochitest-browser-chrome: intermittent ".../suite/browser/...
Status: RESOLVED FIXED
[failure is worked around on c-1.9.1]...
: intermittent-failure
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: seamonkey2.1b1
Assigned To: Ian Neal
:
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on: 335936 495751 484188 573246
Blocks: SmTestFail 438871 488025
  Show dependency treegraph
 
Reported: 2009-05-05 19:16 PDT by Serge Gautherie (:sgautherie)
Modified: 2012-11-26 02:59 PST (History)
1 user (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch [Checkin: Comment 10] (1.71 KB, patch)
2009-05-19 01:53 PDT, neil@parkwaycc.co.uk
kairo: review+
Details | Diff | Splinter Review
(Bv1) Use todo() ftb (1.05 KB, patch)
2009-05-28 17:17 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
move setTimeout() to actually be between tab key and testingAdditional diagnostic [Backout: Comment 50] (834 bytes, patch)
2009-05-30 13:08 PDT, Robert Kaiser
neil: review+
Details | Diff | Splinter Review
(Bv1a) Use todo() ftb (1.24 KB, patch)
2009-07-05 17:50 PDT, Serge Gautherie (:sgautherie)
neil: review-
Details | Diff | Splinter Review
Extra diagnostic [Checkin: Comment 45] (644 bytes, patch)
2009-07-07 06:00 PDT, neil@parkwaycc.co.uk
kairo: review+
Details | Diff | Splinter Review
Additional diagnostic [Backout: Comment 50] (1.00 KB, patch)
2009-07-14 13:39 PDT, neil@parkwaycc.co.uk
kairo: review+
Details | Diff | Splinter Review
(Bv2) Undo 2 non-helping hacks, Use todo_is() ftb (2.21 KB, patch)
2009-08-06 10:30 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
(Bv2a) Undo 3 non-helping hacks, Use todo_is() ftb [Checkin: Comment 50] (2.13 KB, patch)
2009-08-07 03:53 PDT, Serge Gautherie (:sgautherie)
neil: review+
Details | Diff | Splinter Review
Port changes from FF patch v0.1 (3.33 KB, patch)
2010-09-01 15:14 PDT, Ian Neal
no flags Details | Diff | Splinter Review
Port changes from FF patch v0.2 [Checkin: Comment 62] (3.19 KB, patch)
2010-09-02 05:54 PDT, Ian Neal
neil: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2009-05-05 19:16:23 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1241561224.1241566394.16268.gz
Linux comm-central unit test on 2009/05/05 15:07:04

{
chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on tab not activeElement
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on tab again activeElement
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got [object HTMLInputElement], expected [object XULElement]
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on tab while focused still activeElement
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on another tab while focused still activeElement
chrome://mochikit/content/browser/suite/browser/test/browser_feed_tab.js
}
Comment 1 Serge Gautherie (:sgautherie) 2009-05-05 19:33:36 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1241571349.1241576446.28456.gz
Linux comm-central unit test on 2009/05/05 17:55:49
Comment 2 Robert Kaiser 2009-05-06 03:31:52 PDT
Gah, looks like bug 488025 with different symptoms :(
Comment 3 Serge Gautherie (:sgautherie) 2009-05-06 05:26:07 PDT
(In reply to comment #2)
> Gah, looks like bug 488025 with different symptoms :(

Indeed.
Comment 4 Serge Gautherie (:sgautherie) 2009-05-11 18:44:44 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1242073151.1242078292.26779.gz
Linux comm-1.9.1 unit test on 2009/05/11 13:19:11
Comment 5 Serge Gautherie (:sgautherie) 2009-05-16 07:55:56 PDT
Last time it happened on regular box:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1242391906.1242397375.11138.gz
Linux comm-central unit test on 2009/05/15 05:51:46

whereas it seems to happen _very frequently_ on SeaMonkey-Ports, as in:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1242476841.1242480915.5765.gz
Linux comm-1.9.1 unit test on 2009/05/16 05:27:21

May be you could try to set an actual delay in |setTimeout(step3_5, 0);| and/or try bug 488025 comment 6 suggestion?
Or you could disable this test/check for the time being...
Comment 6 Serge Gautherie (:sgautherie) 2009-05-17 19:54:10 PDT
(In reply to comment #5)
> whereas it seems to happen _very frequently_ on SeaMonkey-Ports, as in:

Well, maybe it's more random than not:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1242586360.1242592285.397.gz
Linux comm-central unit test on 2009/05/17 11:52:40
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1242592185.1242596736.7857.gz
Linux comm-central unit test on 2009/05/17 13:29:45
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1242599198.1242603768.19991.gz
Linux comm-central unit test on 2009/05/17 15:26:38
Comment 7 Serge Gautherie (:sgautherie) 2009-05-18 06:40:00 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1242635218.1242639804.14918.gz
Linux comm-central unit test on 2009/05/18 01:26:58
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1242640801.1242645358.24218.gz
Linux comm-central unit test on 2009/05/18 03:00:01
Comment 8 neil@parkwaycc.co.uk 2009-05-19 01:53:09 PDT
Created attachment 378285 [details] [diff] [review]
Proposed patch
[Checkin: Comment 10]

Changing the click position to 9, 9 seems to make the test more reliable.

I had to change the isnot to todo_is because we differ from Firefox (this difference is what bug 462289 is all about).

I added an extra test for good measure.
Comment 9 Robert Kaiser 2009-05-19 10:41:23 PDT
Comment on attachment 378285 [details] [diff] [review]
Proposed patch
[Checkin: Comment 10]

Let's hope this actually clears this intermittent orange now, thanks for investigating!
Comment 10 neil@parkwaycc.co.uk 2009-05-20 01:09:53 PDT
Pushed changeset bd555c5dc66f to comm-central.

Our linux test builds went green for a couple of cycles (before someone inconsiderately checked in some 1.9.1 bustage!)
Comment 11 Serge Gautherie (:sgautherie) 2009-05-24 19:30:52 PDT
Well, same failure still happens :-/

For example:
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1243182039.1243186555.29946.gz
Linux comm-central unit test on 2009/05/24 09:20:39
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1243195135.1243199665.21947.gz
Linux comm-central unit test on 2009/05/24 12:58:55

Running chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js...
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on tab selects tab
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on tab not activeElement
TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on tab again activeElement
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got [object HTMLInputElement], expected [object XULElement]
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on tab while focused still activeElement
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | mouse on another tab while focused still activeElement
}
Comment 12 Serge Gautherie (:sgautherie) 2009-05-24 20:17:19 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1243197291.1243201411.24997.gz
Linux comm-1.9.1 unit test on 2009/05/24 13:34:51
Comment 13 neil@parkwaycc.co.uk 2009-05-25 04:06:39 PDT
Sorry, I can't reproduce this locally :-(
Comment 14 Serge Gautherie (:sgautherie) 2009-05-26 17:46:50 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1243378847.1243383696.28403.gz
Linux comm-central unit test on 2009/05/26 16:00:47
Comment 15 Serge Gautherie (:sgautherie) 2009-05-28 17:17:22 PDT
Created attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb

This happen rather often: let's work around this ftb.
Comment 16 Serge Gautherie (:sgautherie) 2009-05-28 17:20:06 PDT
The failing test does
{
EventUtils.synthesizeKey("VK_TAB", { });
}

Could it be related to bug 335936?
Comment 17 Serge Gautherie (:sgautherie) 2009-05-28 18:57:58 PDT
Comment on attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb


NB: Actually, I could do this for Linux only...
Comment 18 Robert Kaiser 2009-05-29 04:02:28 PDT
(In reply to comment #16)
> Could it be related to bug 335936?

Possibly, but then I still wonder why Firefox doesn't have the same problem.
Comment 19 Robert Kaiser 2009-05-29 04:05:10 PDT
Comment on attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb

1) It doesn't happen everywhere
2) We are not out of options yet, IMHO we should at least try the workaround I hadin place in some earlier iteration of the testcase, where I issued the tab key a second time if we were still on the input element.
Comment 20 Robert Kaiser 2009-05-30 13:08:47 PDT
Created attachment 380638 [details] [diff] [review]
move setTimeout() to actually be between tab key and testingAdditional diagnostic
[Backout: Comment 50]

So, I still could see the failure locally, and I noticed that when I did an if for the active element that tried re-sending a tab key when the input element is still focused, that was pretty consistently triggered but it didn't actually help. As soon as I changed it into a while loop, the test worked and the loop was never triggered.
Moving the key generation before the setTimeout() also helped and is the easier solution, so let's try that one. It looks like the focus works fine, the tab key needs to actually settle.
Comment 21 Robert Kaiser 2009-05-30 15:50:20 PDT
Comment on attachment 380638 [details] [diff] [review]
move setTimeout() to actually be between tab key and testingAdditional diagnostic
[Backout: Comment 50]

Pushed as http://hg.mozilla.org/comm-central/rev/ea695ad6ee9f - let's see how this performs on the test boxes.
Comment 22 Serge Gautherie (:sgautherie) 2009-05-31 07:17:22 PDT
(In reply to comment #21)
> let's see how this performs on the test boxes.

Not enough, but made a difference:
the error changed (a little):
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1243751558.1243756173.22047.gz
Linux comm-1.9.1 unit test on 2009/05/30 23:32:38

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got [object XULElement], expected [object XULElement]
}
Comment 23 Serge Gautherie (:sgautherie) 2009-06-01 06:21:15 PDT
Ah,
{
29   document.getElementById("urlbar").inputField.focus();
31   EventUtils.synthesizeKey("VK_TAB", { });
}
would very much look like newly filed Core bug 495751...
Comment 24 Serge Gautherie (:sgautherie) 2009-06-11 09:34:57 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1244704895.1244710227.7854.gz
Linux comm-1.9.1 unit test on 2009/06/11 00:21:35
Comment 25 Serge Gautherie (:sgautherie) 2009-06-13 11:37:31 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1244908544.1244913200.8473.gz
Linux comm-central unit test on 2009/06/13 08:55:44

This is happening rather often: workaround wanted.
Comment 26 Serge Gautherie (:sgautherie) 2009-07-03 07:08:27 PDT
Comment on attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb

Reviving this workaround, as the others were not enough.
Comment 27 Robert Kaiser 2009-07-03 07:15:34 PDT
Comment on attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb

I don't really like that "patch", it's too lame IMHO.
Comment 28 Serge Gautherie (:sgautherie) 2009-07-03 07:30:27 PDT
(In reply to comment #27)

What else then?
Comment 29 Robert Kaiser 2009-07-03 07:46:08 PDT
(In reply to comment #28)
> (In reply to comment #27)
> 
> What else then?

Still somewhat lame, but IMHO better would be to only make it a todo() when things don't work and make it pass correctly when it does. Unfortunately, there's no maybe_todo() or so.
Comment 30 Serge Gautherie (:sgautherie) 2009-07-05 17:50:55 PDT
Created attachment 386915 [details] [diff] [review]
(Bv1a) Use todo() ftb

Bv1, with comment 17 suggestion(s).
Comment 31 Serge Gautherie (:sgautherie) 2009-07-05 17:53:40 PDT
Comment on attachment 386915 [details] [diff] [review]
(Bv1a) Use todo() ftb


PS: I'll remove the superfluous '{}'.
Comment 32 Robert Kaiser 2009-07-06 11:27:25 PDT
Comment on attachment 386915 [details] [diff] [review]
(Bv1a) Use todo() ftb

better, but could we make it todo_is() and only in case the is() would fail? I know that's cheating the system and I'd like Neil to review that as well, but I think we should be green where we can.
Comment 33 Serge Gautherie (:sgautherie) 2009-07-06 12:41:48 PDT
Comment on attachment 386915 [details] [diff] [review]
(Bv1a) Use todo() ftb


(In reply to comment #32)
> better, but could we make it todo_is() and only in case the is() would fail? I

What would be the gain?
Also, as this bug seems to be some kind of "timing" issue, I have no way to know whether an if() could fail and the following todo() succeed...
I'd rather keep the code simple and safe ... and the reminder visible.

> know that's cheating the system and I'd like Neil to review that as well, but I

Asking him, too.

> think we should be green where we can.

The test will be green any way.
Comment 34 neil@parkwaycc.co.uk 2009-07-07 05:55:16 PDT
Strictly speaking this bug is about Got [object HTMLInputElement], expected [object XULElement] but the current test failures are for Got [object XULElement], expected [object XULElement] which is completely different.
Comment 35 neil@parkwaycc.co.uk 2009-07-07 06:00:29 PDT
Created attachment 387183 [details] [diff] [review]
Extra diagnostic
[Checkin: Comment 45]

This will help us pin down exactly where focus is going :-)
Comment 36 Robert Kaiser 2009-07-11 16:43:32 PDT
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got browser, expected tab
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got [object XULElement], expected [object XULElement]
Comment 37 neil@parkwaycc.co.uk 2009-07-13 05:30:35 PDT
(In reply to comment #36)
> TEST-UNEXPECTED-FAIL |
> chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab
> key to tab activeElement - Got browser, expected tab
> TEST-UNEXPECTED-FAIL |
> chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab
> key to tab activeElement - Got [object XULElement], expected [object
> XULElement]
Hmm... so, there are two possibilities:
1. Focus went straight from the URLbar to the browser (content) - unlikely?
2. Focus never went to the URLbar...
Comment 38 neil@parkwaycc.co.uk 2009-07-14 13:39:26 PDT
Created attachment 388545 [details] [diff] [review]
Additional diagnostic
[Backout: Comment 50]

I hope I don't need too many of these, I'm going to run out of synonyms ;-)
Comment 39 Robert Kaiser 2009-07-19 17:31:26 PDT
TEST-PASS | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | focus URL bar activeElement
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got browser, expected tab
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got [object XULElement], expected [object XULElement]
Comment 40 Serge Gautherie (:sgautherie) 2009-07-20 08:32:23 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey2.0/1248091605.1248097108.13715.gz
Linux comm-1.9.1 unit test on 2009/07/20 05:06:45

... | tab key to tab activeElement - Got input, expected tab
... | tab key to tab activeElement - Got [object HTMLInputElement], expected [object XULElement]
Comment 41 Serge Gautherie (:sgautherie) 2009-07-23 19:58:02 PDT
{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey2.0/1248393507.1248399099.20995.gz
Linux comm-1.9.1 unit test on 2009/07/23 16:58:27

... | tab key to tab activeElement - Got browser, expected tab
... | tab key to tab activeElement - Got [object XULElement], expected [object XULElement]
}

Already much time spent on this and no real solution, (I even wonder whether it got worse).

Still suggesting a todo() for the time being.!.
Comment 42 neil@parkwaycc.co.uk 2009-08-06 07:52:37 PDT
Comment on attachment 386915 [details] [diff] [review]
(Bv1a) Use todo() ftb

Assuming the test really is intermittent, I would like an easier way to notice that the test passed or failed without reading the log. Two alternatives spring to mind: a) if (!linux || document.activeElement == tab1) is(document.activeElement, tab1, ...); b) if (!linux) is(document.activeElement, tab1, ...); else if (document.activeElement != tab1) todo_is(document.activeElement, tab1, ...);
Comment 43 neil@parkwaycc.co.uk 2009-08-06 07:57:16 PDT
Also we should back out all the hacks, since they weren't needed on Mac/Win.
Comment 44 Serge Gautherie (:sgautherie) 2009-08-06 09:21:48 PDT
(In reply to comment #41)
> I even wonder whether it got worse.

(In reply to comment #42)
> Assuming the test really is intermittent

It is (still), though "lately" failure is the rule and success the exception:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey2.0/1249506837.1249513101.1291.gz
Linux comm-1.9.1 unit test on 2009/08/05 14:13:57
mochitest-browser-chrome 444/2/8

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey2.0/1249513083.1249518978.32511.gz
Linux comm-1.9.1 unit test on 2009/08/05 15:58:03
mochitest-browser-chrome 446/0/8
Comment 45 Serge Gautherie (:sgautherie) 2009-08-06 09:48:31 PDT
Comment on attachment 387183 [details] [diff] [review]
Extra diagnostic
[Checkin: Comment 45]


http://hg.mozilla.org/comm-central/rev/1a171a63a9a3
Comment 46 Serge Gautherie (:sgautherie) 2009-08-06 10:30:39 PDT
Created attachment 392970 [details] [diff] [review]
(Bv2) Undo 2 non-helping hacks, Use todo_is() ftb

Bv1a, with comment 43 and comment 43 suggestion(s).

Past: I assume executeSoon() would have made no difference.
Future: if todo_is() does intermittently fail then we'll know it's a timing issue.
Comment 47 neil@parkwaycc.co.uk 2009-08-06 12:53:11 PDT
Do we even need the 3_5 hack?
Comment 48 Serge Gautherie (:sgautherie) 2009-08-07 03:53:05 PDT
Created attachment 393159 [details] [diff] [review]
(Bv2a) Undo 3 non-helping hacks, Use todo_is() ftb
[Checkin: Comment 50]

Bv2, plus undo bug 488025 hack too.
Comment 49 neil@parkwaycc.co.uk 2009-08-07 08:50:12 PDT
Comment on attachment 393159 [details] [diff] [review]
(Bv2a) Undo 3 non-helping hacks, Use todo_is() ftb
[Checkin: Comment 50]

>+    // Check local name too to help diagnose bug 491624.
>+    todo_is(document.activeElement.localName, "tab",
>+            "tab key to tab activeElement (bug 491624: name = " +
>+              document.activeElement.localName + ")");
>+    todo_is(document.activeElement, tab1,
>+            "tab key to tab activeElement (bug 491624: object = " +
>+              document.activeElement + ")");
It's annoying that todo_is doesn't do that for you :-(
Comment 50 Serge Gautherie (:sgautherie) 2009-08-07 10:37:01 PDT
Comment on attachment 393159 [details] [diff] [review]
(Bv2a) Undo 3 non-helping hacks, Use todo_is() ftb
[Checkin: Comment 50]


http://hg.mozilla.org/comm-central/rev/9baa4ac307b3


(In reply to comment #49)
> It's annoying that todo_is doesn't do that for you :-(

Yeah, no support for "random" behaviors in the mochitest (SimpleTest) harness:
Maybe this is not as needed as for the reftests?
Comment 51 Serge Gautherie (:sgautherie) 2009-08-09 05:18:41 PDT
Fwiw, see bug 507046 as a possible workaround...
Comment 52 Robert Kaiser 2009-08-09 08:25:38 PDT
(In reply to comment #51)
> Fwiw, see bug 507046 as a possible workaround...

Doesn't make much sense, as what we want to test is that navigation with the tab key gets us to the correct tab.
Comment 53 Serge Gautherie (:sgautherie) 2009-12-07 17:21:28 PST
Fwiw, note
http://hg.mozilla.org/comm-central/diff/e29bae488b4d/suite/browser/test/browser/browser_bug462289.js
we do have some "timing weirdness" here...
Comment 54 Robert Kaiser 2009-12-20 07:42:28 PST
Does this still happen?

I think I give up on it as long as I have no clue what's going on there.
Comment 55 Serge Gautherie (:sgautherie) 2009-12-20 10:17:07 PST
(In reply to comment #54)
> Does this still happen?

I don't know: I'm not checking random failures anymore until bug 534647 is fixed :-|
Comment 56 Ian Neal 2010-09-01 15:14:34 PDT
Created attachment 471305 [details] [diff] [review]
Port changes from FF patch v0.1

This patch:
* Ports changes made in Firefox's version of this test and removed all todos.
Comment 57 neil@parkwaycc.co.uk 2010-09-01 16:12:24 PDT
Comment on attachment 471305 [details] [diff] [review]
Port changes from FF patch v0.1

>-  tab1 = gBrowser.addTab();
>-  tab2 = gBrowser.addTab();
>+  tab1 = gBrowser.addTab("about:blank", {skipAnimation: true});
>+  tab2 = gBrowser.addTab("about:blank", {skipAnimation: true});
We don't actually support animation...

>-  is(gBrowser.selectedTab, tab1, "mouse on tab selects tab");
Why has this been removed?

>   document.getElementById("urlbar").inputField.focus();
>-  EventUtils.synthesizeKey("VK_TAB", { });
>+  while (focus_in_navbar())
>+    EventUtils.synthesizeKey("VK_TAB", { });
Out of interest, do we still need the .inputField. bit?
Comment 58 Ian Neal 2010-09-01 17:55:00 PDT
(In reply to comment #57)
> Comment on attachment 471305 [details] [diff] [review]
> Port changes from FF patch v0.1
> 
> >-  tab1 = gBrowser.addTab();
> >-  tab2 = gBrowser.addTab();
> >+  tab1 = gBrowser.addTab("about:blank", {skipAnimation: true});
> >+  tab2 = gBrowser.addTab("about:blank", {skipAnimation: true});
> We don't actually support animation...
Hmmm, I think I got confused about which bindings we extend and which we replace.
> 
> >-  is(gBrowser.selectedTab, tab1, "mouse on tab selects tab");
> Why has this been removed?
To match toolkit, but I can re-instate.
> 
> >   document.getElementById("urlbar").inputField.focus();
> >-  EventUtils.synthesizeKey("VK_TAB", { });
> >+  while (focus_in_navbar())
> >+    EventUtils.synthesizeKey("VK_TAB", { });
> Out of interest, do we still need the .inputField. bit?
I will double check.
Somehow I managed to upload an old version of the patch, so will upload an updated one tomorrow.
Comment 59 Ian Neal 2010-09-02 05:54:44 PDT
Created attachment 471469 [details] [diff] [review]
Port changes from FF patch v0.2 [Checkin: Comment 62]

Changes since v0.1:
* Removed animation changes;
* Re-added "mouse on tab selects tab" code;
* Fixed last todo - this works on linux, might need tweaking for other platforms.
Comment 60 neil@parkwaycc.co.uk 2010-09-02 06:15:00 PDT
Comment on attachment 471469 [details] [diff] [review]
Port changes from FF patch v0.2 [Checkin: Comment 62]

I happened to notice that some of the todo_is are now isnot. Does that mean that we've decided that the tested behaviour is now correct?
Comment 61 Ian Neal 2010-09-02 09:10:52 PDT
(In reply to comment #60)
> Comment on attachment 471469 [details] [diff] [review]
> Port changes from FF patch v0.2
> 
> I happened to notice that some of the todo_is are now isnot. Does that mean
> that we've decided that the tested behaviour is now correct?

Well it matches what Firefox have as the result of tested behaviour...
Comment 62 Ian Neal 2010-09-02 12:29:58 PDT
Comment on attachment 471469 [details] [diff] [review]
Port changes from FF patch v0.2 [Checkin: Comment 62]

http://hg.mozilla.org/comm-central/rev/4b9cb609548d
Comment 63 Serge Gautherie (:sgautherie) 2010-09-02 16:37:01 PDT
Good :-) Would want this test update on c-1.9.1 too...
Comment 64 Ian Neal 2010-09-02 16:48:06 PDT
(In reply to comment #63)
> Good :-) Would want this test update on c-1.9.1 too...

Could we get a tracking bug to cover those that we want to backport to c-1.9.1 so that we don't forget them?
Comment 65 Serge Gautherie (:sgautherie) 2010-09-02 17:13:09 PDT
(In reply to comment #64)

Why not use this very bug?

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