Boris Zbarsky [:bzbarsky]
	Comment 5 • 14 years ago

Comment on attachment 428263 [details] [diff] [review]
patch

r=bzbarsky

	Bernd Assignee
	Comment 6 • 14 years ago

Comment on attachment 428263 [details] [diff] [review]
patch

the patch is bogus, it fails at the second reftest, I can't figure how I got a reftest run.

While reading I think that if http://mxr.mozilla.org/mozilla-central/source/layout/tables/nsTableColGroupFrame.cpp#255 the assertion triggers we will access deleted memory on the next run.

	Bernd Assignee
	Comment 7 • 14 years ago

Boris, the only differenz  to the previous patch that you reviewed is the 
PRBool contentRemoval = PR_FALSE; variable.
And yes it passed the try server.

	Boris Zbarsky [:bzbarsky]
	Comment 8 • 14 years ago

Comment on attachment 429949 [details] [diff] [review]
revised patch

r=bzbarsky

	Bernd Assignee
	Comment 9 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/ccc04ad335a8

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11)
	Comment 10 • 14 years ago

http://hg.mozilla.org/mozilla-central/log/eaf1574b10b6/layout/reftests/bugs/281241-2.html
fails with the HTML5 parser. The test has a script in a colgroup, which is not nice.

	Bernd Assignee
	Comment 11 • 14 years ago

> which is not nice.

I have difficulties to parse this:

Is this a parser problem that we exhibit with this testcase? Or do you object to this little evil testcase which uses a layout break point to test incremental reflow. We do this since ages (http://www.mozilla.org/newlayout/doc/fosdem2004/slide21.html) and it does here exactly what I want.

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11)
	Comment 12 • 14 years ago

Putting a <script> tag between the <colgroup> and <col> tags results in two colgroups in the DOM according to the HTML5 parsing algorithm, because the <script> implicitly closes the first colgroup (http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#parsing-main-incolgroup) and the <col> implies another (http://www.whatwg.org/specs/web-apps/current-work/multipage/tokenization.html#parsing-main-intable).

Since two colgroups appear in the DOM, the layout doesn't match the reference.

Can this be tested without putting a script at that particular point? Would an XHTML test exercise the layout code equally well?

	Bernd Assignee
	Comment 14 • 14 years ago

Comment on attachment 431836 [details] [diff] [review]
Turn the HTML5-incompatible test case into XHTML

thats what I also came up with, but I was yesterday not able to attach it. Thank you.

	David Baron :dbaron:
	Comment 15 • 14 years ago

In XHTML, doesn't the script end up executing much later than needed for this testcase?

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11)
	Comment 16 • 14 years ago

(In reply to comment #15)
> In XHTML, doesn't the script end up executing much later than needed for this
> testcase?

No, as far as I can tell:
http://hsivonen.iki.fi/test/moz/script-execution.xhtml

	Henri Sivonen (:hsivonen) (away from Bugzilla until 2023-09-11)
	Comment 17 • 14 years ago

Pushed the test case fix:
http://hg.mozilla.org/mozilla-central/rev/142807456168