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

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Serge Gautherie (:sgautherie)]

[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

[:Paolo Amadini]

Seems like portions of bug 521557 should be ported to the 1.9.2 branch?

Comment 2

[Serge Gautherie (:sgautherie)]

(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

[:Paolo Amadini]

(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.

Comment 4

[Serge Gautherie (:sgautherie)]

(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

[:Paolo Amadini]

(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?

Comment 6

[Serge Gautherie (:sgautherie)]

(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?

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (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.

Comment 8

[:Paolo Amadini]

(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

[[e-mail]]

Shouldn't you use executeSoon instead of setTimeout?

Comment 10

[[e-mail]]

Sorry, I hadn't read the patch.

Comment 11

[Serge Gautherie (:sgautherie)]

Attachment: (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.)

Comment 12

[[e-mail]]

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? ;-)

Comment 13

[Serge Gautherie (:sgautherie)]

Attachment: (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.

Comment 14

[Serge Gautherie (:sgautherie)]

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


http://hg.mozilla.org/mozilla-central/rev/14b0f9623642

Comment 15

[Serge Gautherie (:sgautherie)]

(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.