330623 - document.all warnings fill up the event queue

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

If you go to http://classad.163.com/html/area/255/index.html (WARNING, CRASHES) firefox hangs. It seems like the script on the page is using document.all to animate a banner, and this causes so many warnings that we just choke.

There are IMHO two problems here. First of all we should not issue this many document.all warnings. One per page should be enough (or script context or whatever is easy enough to do).

Second, we should not fall over just because a script produces a lot of warnings. Either we should change the console interfaces to allow all so-far created messages to be collected at the same time, or we should make sure not to send out more then X number of notifications at a time.

Comment 1

[timeless]

note that we already have a proposed new api for the consoleservice, so please don't invent another.

that said, one per something definitely sounds like a good idea, i think i had asked for that somewhere, although i doubt i'd ever find where.

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Only warn once

Since when sicking and I looked at this, the event queue was absolutely *stuffed* with these warning messages, I'm hypothesizing that only warning about document.all usage once will fix the hange he was seeing. It's worth noting that the site is still asking for exponential behavior (f(); function f() { setInterval('f()');) so it'll peg your CPU a little bit.

This patch makes us only warn once, since the JS engine will only need to resolve the document.all property once, so we shouldn't flood the event queue with warnings.

Comment 3

[Martijn Wargers (dead)]

(In reply to comment #2)
> It's worth
> noting that the site is still asking for exponential behavior (f(); function
> f() { setInterval('f()');) so it'll peg your CPU a little bit.
That's bug 261633, btw.

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 215189 [details] [diff] [review]
Only warn once

r+sr=jst

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.