277208 - A lot of nested marquees can make Mozilla freeze

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20050103 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20050103 Firefox/1.0+

The testcase that I will attach gives a nice/silly effect in IE6, but in Mozilla
it makes the browser freeze and completely unusable until I close it.
This was mentioned at:
http://forums.mozillazine.org/viewtopic.php?t=190367

It consists of a lot of nested <marquee> tags.
Either, Mozilla should give a warning that a script is causing Mozilla to run
slowly, etc and give the user the capability to shut the script down or Mozilla
should not support <marquee> deeply nested marquees. This could be done in
html.css, perhaps? Something like: marquee marquee marquee marquee
{-moz-binding:none;} , maybe?

Reproducible: Always

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

Comment 2

[Martijn Wargers (dead)]

Attachment: patch

Oops, I see now that bug 239840 is basically the same bug.
Boris, is this something worth considering?
The nesting level where I disable marquee bindings here is rather arbritary.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So... what's the real performance problem here?  Is it really marquee-specific?
Or would it happen with any nested set of scrollframes?

Comment 4

[Martijn Wargers (dead)]

I'm not sure what you mean. The testcase doesn't freeze when I disable javascript.
Do you want to see a testcase with a minimal marquee.xml example that still
freezes Mozilla?

Comment 5

[Martijn Wargers (dead)]

Attachment: Minimised testcase

This testcase -based on the marquee code- doesn't need javascript to make it
hang/freeze Mozilla. It freezes Mozilla for 10 seconds or so. 
but when I add another <marquee>, it will freeze Mozilla much longer. Every
added <marquee> seems to increase the freezing time exponentially.

Comment 6

[Martijn Wargers (dead)]

Attachment: Testcase2

The xbl binding is necessary to trigger the bug. In other words, replacing the
<marquee> with the <xul:hbox style="margin: 0 100%;"> (entirely or partly)
doesn't trigger the bug.

Comment 7

[Martijn Wargers (dead)]

Attachment: Testcase1

sorry, the testcase were wrong.

Comment 8

[Martijn Wargers (dead)]

Attachment: Testcase2

Testcase2 is basically the result content of the xbl binding. This is also very
slow. So xul hboxes -with 100% margin- nested inside inline boxes are very slow
with rendering.

Comment 9

[Martijn Wargers (dead)]

Ok, testcase2 is not slow for me in:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a4) Gecko/20040928
Firefox/0.9.1+ 07:26am
But it is extremely slow for me in:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a4) Gecko/20040929
Firefox/0.9.1+ 08:09am

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-09-28+07%3A00%3A00&maxdate=2004-09-29+08%3A00%3A00&cvsroot=%2Fcvsroot

Maybe a regression from bug 258513?

cc-ing you, Boris. You might be interested in this.
This regression I mention here, is probably not the cause of the bug I initially
reported, though.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Yes, the box landing seems to be a very likely cause for this...  There are also
some scrollframe changes in that range, but they look innocuous.

Comment 11

[Martijn Wargers (dead)]

Attachment: patchv1

Well, replacing the xul:hbox by a html:div style="display:table" seems to fix
this issue and does not seem to have any other side-effects.

Comment 12

[Martijn Wargers (dead)]

There is at least one large difference between xul:hbox and display:table that
could influence marquees: text in the xul:hbox widens the xul:hbox even with
spaces in the text. That's not the case with display:table. There, the text gets
split across multiple lines. (maybe that's also what xul:hbox should be doing?
Where can I find info on that?)
But there's a rule in the xbl-marquee.xml that's preventing that:
this.innerDiv.style.whiteSpace = "nowrap";
http://lxr.mozilla.org/seamonkey/source/layout/style/xbl-marquee/xbl-marquee.xml#229

So that's why I still think that display:table would be fine to use.
The patch also seems to fix bug 269257, by the way.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 187631 [details] [diff] [review]
patchv1

Seems reasonable, though I'd like a followup bug on the box perf issue, with
the [reflow-refactor] annotation in the status whiteboard.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Checked in on the trunk...

Comment 15

[Martijn Wargers (dead)]

(In reply to comment #13)
> Seems reasonable, though I'd like a followup bug on the box perf issue, with
> the [reflow-refactor] annotation in the status whiteboard.
Ok, I filed bug 307763.

Comment 16

[Martijn Wargers (dead)]

This bug is fixed. The follow-up bug is bug 307763.
The first testcase isn't behaving exactly like IE6 (Mozilla is behaving a bit
erraticly in the beginning), but I'm not even sure if that's something worth
filing a bug for (with such a complicated testcase and such a special circumstance).

Comment 17

[Andrew Schultz]

*** Bug 309861 has been marked as a duplicate of this bug. ***

Comment 18

[Andrew Schultz]

*** Bug 309861 has been marked as a duplicate of this bug. ***

Comment 19

[Martijn Wargers (dead)]

*** Bug 339677 has been marked as a duplicate of this bug. ***

Comment 20

[Jesse Ruderman]

See also bug 239840, "hang when many nested <marquee> tags are used. exponential time increase".