577508 - HTML5 parser regresses bug 253951

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

The HTML5 parser's FlushPendingNotifications doesn't start layout on layout flushes.  As a result, bug 253951 is reintroduced.

I wish we had a way to write a test for this.  :(

Maybe we should think of a better way to fix that bug than flushing?

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: Make FlushPendingNotifications call nsContentSink::StartLayout

Not sure yet if this starts layout more often than necessary.

Comment 2

[Henri Sivonen (:hsivonen)]

Comment on attachment 457871 [details] [diff] [review]
Make FlushPendingNotifications call nsContentSink::StartLayout

This early layout start is triggered on only two pages in the tp4 pageset, so I guess this code doesn't run too often to be a problem.

Comment 3

[Henri Sivonen (:hsivonen)]

Comment on attachment 457871 [details] [diff] [review]
Make FlushPendingNotifications call nsContentSink::StartLayout

Oops. This patch causes unit test badness. :-(

Comment 4

[Boris Zbarsky [:bzbarsky]]

How so?

Comment 5

[Henri Sivonen (:hsivonen)]

(In reply to comment #4)
> How so?

The patch starts layout but doesn't flush ops. There are mochitests that want to read colors but get all white data back if layout has started before ops have been flushed.

Comment 6

[Henri Sivonen (:hsivonen)]

Well, at least *a* mochitest:
http://mxr-test.konigsberg.mozilla.org/mozilla-central/source/layout/base/tests/test_bug469170.html?force=1

Comment 7

[Henri Sivonen (:hsivonen)]

Attachment: Make FlushPendingNotifications call nsContentSink::StartLayout, run a test on onload

The same patch as before but dealing with yet another test that used a data: URL withou onload.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 472721 [details] [diff] [review]
Make FlushPendingNotifications call nsContentSink::StartLayout, run a test on onload

r=me

Comment 9

[Henri Sivonen (:hsivonen)]

http://hg.mozilla.org/mozilla-central/rev/353a0ee16d78