Closed Bug 281241 Opened 15 years ago Closed 10 years ago

dynamical inserted/appended columns need to replace span defined anonymous columns

Categories

(Core :: Layout: Tables, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bernd_mozilla, Assigned: bernd_mozilla)

References

Details

(Keywords: testcase)

Attachments

(4 files, 1 obsolete file)

 
Attached file testcase
Summary: dynamical inserted/appended column need to replace span define anonymous columns → dynamical inserted/appended columns need to replace span defined anonymous columns
Attached file removal testcase
Depends on: 302118
Duplicate of this bug: 493564
Attached patch patch (obsolete) — Splinter Review
Attachment #428263 - Flags: review?(bzbarsky)
Comment on attachment 428263 [details] [diff] [review]
patch

r=bzbarsky
Attachment #428263 - Flags: review?(bzbarsky) → review+
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.
Attachment #428263 - Attachment is obsolete: true
Attachment #428263 - Flags: review+
Attached patch revised patchSplinter Review
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.
Attachment #429949 - Flags: review?(bzbarsky)
Comment on attachment 429949 [details] [diff] [review]
revised patch

r=bzbarsky
Attachment #429949 - Flags: review?(bzbarsky) → review+
http://hg.mozilla.org/mozilla-central/rev/ccc04ad335a8
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
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.
> 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.
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?
Attachment #431836 - Flags: review?(bernd_mozilla)
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.
Attachment #431836 - Flags: review?(bernd_mozilla) → review+
In XHTML, doesn't the script end up executing much later than needed for this testcase?
(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
You need to log in before you can comment on or make changes to this bug.