[SeaMonkey 2.1 !?] mochitest-browser-chrome: browser_bug295977_autoscroll_overflow.js can cause browser_bug471962.js to fail

RESOLVED FIXED in mozilla1.9.3a1

Status

()

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Blocks: 1 bug)

Trunk
mozilla1.9.3a1
x86
Windows 2000
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a1pre) Gecko/20091218
SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)

Comment 1

9 years ago
Seems like portions of bug 521557 should be ported to the 1.9.2 branch?
Depends on: 521557
(Assignee)

Comment 2

9 years ago
(In reply to comment #1)
> Seems like portions of bug 521557 should be ported to the 1.9.2 branch?

Maybe ... though I'm not sure why "1.9.2 Linux" would help "1.9.3 Windows"?

Comment 3

9 years ago
(In reply to comment #2)
> (In reply to comment #1)
> > Seems like portions of bug 521557 should be ported to the 1.9.2 branch?
> 
> Maybe ... though I'm not sure why "1.9.2 Linux" would help "1.9.3 Windows"?

The bug title is misleading in this context; the patch fixes both the problem
on Linux and the issue reported as bug 512100 (Fix dependency between
"browser_keyevents_during_autoscrolling.js" and "browser_bug471962.js").

By the way, I'm not sure if the patch there applies cleanly to the branch,
but resolving the potential conflicts should be trivial.

As for the 1.9.2 / 1.9.3 difference, doesn't comm-central depend on the
mozilla1.9.2 branch? I remember comm-central used to depend on the
mozilla1.9.1 branch before the comm-1.9.1 branch was created.
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)
> As for the 1.9.2 / 1.9.3 difference, doesn't comm-central depend on the
> mozilla1.9.2 branch? I remember comm-central used to depend on the
> mozilla1.9.1 branch before the comm-1.9.1 branch was created.

No (and yes): current trees are "c-1.9.1 + m-1.9.1" and "c-c + m-c".
Thunderbird (at least) is setting up a "c-1.9.2 + m-1.9.2" tree, iiuc ;->

Comment 5

9 years ago
(In reply to comment #4)
> (In reply to comment #3)
> > As for the 1.9.2 / 1.9.3 difference, doesn't comm-central depend on the
> > mozilla1.9.2 branch? I remember comm-central used to depend on the
> > mozilla1.9.1 branch before the comm-1.9.1 branch was created.
> 
> No (and yes): current trees are "c-1.9.1 + m-1.9.1" and "c-c + m-c".
> Thunderbird (at least) is setting up a "c-1.9.2 + m-1.9.2" tree, iiuc ;->

Well, in this case I think the cause of this failure should be sought in a
newly introduced test that doesn't clean up properly, with a pattern similar
to the case solved by bug 521557. It's strange, however, that the failure
happens on comm-central and not mozilla-central. Maybe a comm-central
specific browser-chrome test?
(Assignee)

Comment 6

9 years ago
(In reply to comment #0)
> [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.3a1pre) Gecko/20091218
> SeaMonkey/2.1a1pre] (home, optim default) (W2Ksp4)

{
"browser_bug471962.js | TypeError: innerFrame.contentDocument.getElementById("postForm") is null"
}

(In reply to comment #5)
> It's strange, however, that the failure
> happens on comm-central and not mozilla-central. Maybe a comm-central
> specific browser-chrome test?

It turned out to be m-c bug 295977 test.
SeaMonkey 2.1 has no test boxes yet...
Maybe it happens on my computer only because it would be faster (or something) than the Firefox boxes?
Assignee: nobody → sgautherie.bz
Blocks: 295977
No longer blocks: 470184
Status: NEW → ASSIGNED
Component: Download Manager → XUL Widgets
No longer depends on: 471962, 521557
QA Contact: download.manager → xul.widgets
Summary: [SeaMonkey 2.1] mochitest-browser-chrome: "browser_bug471962.js | TypeError: innerFrame.contentDocument.getElementById("postForm") is null" → [SeaMonkey 2.1 !?] mochitest-browser-chrome: browser_bug295977_autoscroll_overflow.js can cause browser_bug471962.js to fail
Target Milestone: --- → mozilla1.9.3a1
(Assignee)

Comment 7

9 years ago
Created attachment 418534 [details] [diff] [review]
(Av1) Use setTimeout(, 0) as a workaround

I assume this issue is caused by the tab clean-up which happens just before.
I wonder if there would be a cleaner way to handle it? Like an event or something.
Attachment #418534 - Flags: review?(neil)

Comment 8

9 years ago
(In reply to comment #7)
> I assume this issue is caused by the tab clean-up which happens just before.
> I wonder if there would be a cleaner way to handle it? Like an event or
> something.

I wondered the same thing :-) Actually, the following line...

gBrowser.addTab().linkedBrowser.stop();

...was introduced in bug 521557 to work around the problem, but when I asked
why it worked, got no response ;-) As it turns out, it is not enough.

If the setTimeout call (analogue to my original solution as well) works,
or another cleaner solution is found, I think the line above can be safely
reverted to just:

gBrowser.addTab();

Comment 9

9 years ago
Shouldn't you use executeSoon instead of setTimeout?
Sorry, I hadn't read the patch.
(Assignee)

Comment 11

9 years ago
Created attachment 418556 [details] [diff] [review]
(Av1a) Use waitForFocus()

Av1, cleaner and more documented.

(In reply to comment #8)
> gBrowser.addTab().linkedBrowser.stop();

The failure can still happen with waitForFocus(): '.linkedBrowser.stop()' needs to remain.
(I didn't check if setTimeout(, 0) would fix this by itself.)
Attachment #418534 - Attachment is obsolete: true
Attachment #418556 - Flags: review?(neil)
Attachment #418534 - Flags: review?(neil)
Comment on attachment 418556 [details] [diff] [review]
(Av1a) Use waitForFocus()

>     gBrowser.addTab().linkedBrowser.stop();
Note that our addTab doesn't correctly check for a missing argument... does changing this to addTab("about:blank") make any difference?

>+    // waitForFocus() fixes it.
>+    waitForFocus(finish);
Is it me or is that comment too obvious? ;-)
(Assignee)

Comment 13

9 years ago
Created attachment 418578 [details] [diff] [review]
(Av1b) Use waitForFocus()
[Checkin: Comment 14]

Av1a, with comment 12 suggestion(s).

(In reply to comment #12)
> does changing this to addTab("about:blank") make any difference?

In this round, I couldn't make it fail even with just gBrowser.addTab(). (It may depend on the load of my computer or something.)
Then I'd say let's try with "about:blank" and we'll know what to do if it randomly fails again.
Attachment #418556 - Attachment is obsolete: true
Attachment #418578 - Flags: review?(neil)
Attachment #418556 - Flags: review?(neil)

Updated

9 years ago
Attachment #418578 - Flags: review?(neil) → review+
(Assignee)

Comment 14

9 years ago
Comment on attachment 418578 [details] [diff] [review]
(Av1b) Use waitForFocus()
[Checkin: Comment 14]


http://hg.mozilla.org/mozilla-central/rev/14b0f9623642
Attachment #418578 - Attachment description: (Av1b) Use waitForFocus(). → (Av1b) Use waitForFocus() [Checkin: Comment 14]
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Depends on: 536940
(Assignee)

Comment 15

9 years ago
(In reply to comment #12)
> >     gBrowser.addTab().linkedBrowser.stop();
> Note that our addTab doesn't correctly check for a missing argument...

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