Closed Bug 491624 Opened 11 years ago Closed 9 years ago

[SeaMonkey, Linux] mochitest-browser-chrome: intermittent ".../suite/browser/test/browser_bug462289.js | tab key to tab activeElement - Got [object HTMLInputElement], expected [object XULElement]"

Categories

(SeaMonkey :: UI Design, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b1

People

(Reporter: sgautherie, Assigned: iann_bugzilla)

References

(Depends on 2 open bugs, Blocks 1 open bug, )

Details

(Keywords: intermittent-failure, Whiteboard: [failure is worked around on c-1.9.1][cc-orange] [Bv2a: fixed-seamonkey2.0b2])

Attachments

(6 files, 4 obsolete files)

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
}
Gah, looks like bug 488025 with different symptoms :(
(In reply to comment #2)
> Gah, looks like bug 488025 with different symptoms :(

Indeed.
Blocks: 488025
No longer blocks: 484188
Depends on: 484188
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...
Assignee: nobody → kairo
Summary: [SeaMonkey] 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/test/browser_bug462289.js | tab key to tab activeElement - Got [object HTMLInputElement], expected [object XULElement]"
(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
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.
Assignee: kairo → neil
Status: NEW → ASSIGNED
Attachment #378285 - Flags: review?(kairo)
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!
Attachment #378285 - Flags: review?(kairo) → review+
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!)
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: in-testsuite+
Target Milestone: --- → seamonkey2.0b1
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
}
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Sorry, I can't reproduce this locally :-(
Attached patch (Bv1) Use todo() ftb (obsolete) — Splinter Review
This happen rather often: let's work around this ftb.
Attachment #380297 - Flags: review?(kairo)
The failing test does
{
EventUtils.synthesizeKey("VK_TAB", { });
}

Could it be related to bug 335936?
Status: REOPENED → ASSIGNED
Depends on: 335936
Blocks: SmTestFail
Comment on attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb


NB: Actually, I could do this for Linux only...
(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 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.
Attachment #380297 - Flags: review?(kairo) → review-
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.
Assignee: neil → kairo
Attachment #380297 - Attachment is obsolete: true
Attachment #380638 - Flags: review?(neil)
Attachment #380638 - Flags: review?(neil) → review+
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.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
(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]
}
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ah,
{
29   document.getElementById("urlbar").inputField.focus();
31   EventUtils.synthesizeKey("VK_TAB", { });
}
would very much look like newly filed Core bug 495751...
Status: REOPENED → ASSIGNED
Depends on: 495751
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.
Attachment #378285 - Attachment description: Proposed patch → Proposed patch [Checkin: Comment 10]
Attachment #380638 - Attachment description: move setTimeout() to actually be between tab key and testing → move setTimeout() to actually be between tab key and testing [Checkin: Comment 21]
Comment on attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb

Reviving this workaround, as the others were not enough.
Attachment #380297 - Attachment is obsolete: false
Attachment #380297 - Flags: review- → review?(kairo)
Comment on attachment 380297 [details] [diff] [review]
(Bv1) Use todo() ftb

I don't really like that "patch", it's too lame IMHO.
(In reply to comment #27)

What else then?
(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.
Attached patch (Bv1a) Use todo() ftb (obsolete) — Splinter Review
Bv1, with comment 17 suggestion(s).
Attachment #380297 - Attachment is obsolete: true
Attachment #386915 - Flags: review?
Attachment #380297 - Flags: review?(kairo)
Attachment #386915 - Flags: review? → review?(kairo)
Comment on attachment 386915 [details] [diff] [review]
(Bv1a) Use todo() ftb


PS: I'll remove the superfluous '{}'.
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 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.
Attachment #386915 - Flags: review?(neil)
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.
This will help us pin down exactly where focus is going :-)
Attachment #387183 - Flags: review?(kairo)
Attachment #387183 - Flags: review?(kairo) → review+
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]
(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...
I hope I don't need too many of these, I'm going to run out of synonyms ;-)
Attachment #388545 - Flags: review?(kairo)
Attachment #388545 - Flags: review?(kairo) → review+
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]
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]
{
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.!.
Attachment #386915 - Flags: review?(neil) → review-
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, ...);
Also we should back out all the hacks, since they weren't needed on Mac/Win.
(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 on attachment 387183 [details] [diff] [review]
Extra diagnostic
[Checkin: Comment 45]


http://hg.mozilla.org/comm-central/rev/1a171a63a9a3
Attachment #387183 - Attachment description: Extra diagnostic → Extra diagnostic [Checkin: Comment 45]
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.
Attachment #386915 - Attachment is obsolete: true
Attachment #392970 - Flags: review?(neil)
Attachment #386915 - Flags: review?(kairo)
Do we even need the 3_5 hack?
Bv2, plus undo bug 488025 hack too.
Attachment #392970 - Attachment is obsolete: true
Attachment #393159 - Flags: review?(neil)
Attachment #392970 - Flags: review?(neil)
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 :-(
Attachment #393159 - Flags: review?(neil) → review+
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?
Attachment #393159 - Attachment description: (Bv2a) Undo 3 non-helping hacks, Use todo_is() ftb → (Bv2a) Undo 3 non-helping hacks, Use todo_is() ftb [Checkin: Comment 50]
Whiteboard: [orange] → [failure is work around] [orange]
Target Milestone: seamonkey2.0b1 → seamonkey2.0b2
Whiteboard: [failure is work around] [orange] → [failure is worked around] [orange]
Attachment #388545 - Attachment description: Additional diagnostic → Additional diagnostic [Backout: Comment 50]
Attachment #380638 - Attachment description: move setTimeout() to actually be between tab key and testing [Checkin: Comment 21] → move setTimeout() to actually be between tab key and testingAdditional diagnostic [Backout: Comment 50]
Fwiw, see bug 507046 as a possible workaround...
(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.
Does this still happen?

I think I give up on it as long as I have no clue what's going on there.
Assignee: kairo → nobody
Target Milestone: seamonkey2.0b2 → ---
(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 :-|
Whiteboard: [failure is worked around] [orange] → [failure is worked around] [orange] [Bv2a: fixed-seamonkey2.0b2]
Status: ASSIGNED → NEW
Attached patch Port changes from FF patch v0.1 (obsolete) — Splinter Review
This patch:
* Ports changes made in Firefox's version of this test and removed all todos.
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Attachment #471305 - Flags: review?(neil)
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?
(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.
Attachment #471305 - Flags: review?(neil)
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.
Attachment #471305 - Attachment is obsolete: true
Attachment #471469 - Flags: review?(neil)
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?
Attachment #471469 - Flags: review?(neil) → review+
(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 on attachment 471469 [details] [diff] [review]
Port changes from FF patch v0.2 [Checkin: Comment 62]

http://hg.mozilla.org/comm-central/rev/4b9cb609548d
Attachment #471469 - Attachment description: Port changes from FF patch v0.2 → Port changes from FF patch v0.2 [Checkin: Comment 62]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b1
Good :-) Would want this test update on c-1.9.1 too...
Whiteboard: [failure is worked around] [orange] [Bv2a: fixed-seamonkey2.0b2] → [failure is worked around on c-1.9.1] [orange] [Bv2a: fixed-seamonkey2.0b2]
(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?
(In reply to comment #64)

Why not use this very bug?
Depends on: 573246
Whiteboard: [failure is worked around on c-1.9.1] [orange] [Bv2a: fixed-seamonkey2.0b2] → [failure is worked around on c-1.9.1] [Bv2a: fixed-seamonkey2.0b2]
Whiteboard: [failure is worked around on c-1.9.1] [Bv2a: fixed-seamonkey2.0b2] → [failure is worked around on c-1.9.1][cc-orange] [Bv2a: fixed-seamonkey2.0b2]
You need to log in before you can comment on or make changes to this bug.