Closed Bug 374297 Opened 13 years ago Closed 12 years ago

[FIX]"Wrong parent style context" involving table pseudo frames


(Core :: CSS Parsing and Computation, defect, P1)






(Reporter: mats, Assigned: bzbarsky)



(Keywords: testcase)


(4 files)

"Wrong parent style context" involving table pseudo frames

I don't know if it's the VerifyContextParent() code

that is wrong or if it's the frame constructor that needs to resolve the style
using GetParentStyleContextFrame() rather than GetParent(), or something else...
What is the correct style context tree for these frames?
Mats, do you have a testcase that gives you that frame tree?  I wonder whether bug 323656 helps here.
Depends on: 323656
The testcases in bug 307992 and other tests of that nature does this a lot.
Yeah, the patch in bug 323656 fixes the ones in that bug.
Now that bug bug 323656 is fixed, is this still a problem?
Attached file Testcase
frame: Table(table)(0) (0x165e9a8) style: 0x165e868 {}
Wrong parent style context:  style: 0x165a408 {}
should be using:  style: 0x165a6c8 :-moz-cell-content {}
Keywords: testcase
Depends on: 377603
Ah, indeed.  The patch for bug 323656 missed a case.  Filed bug 377603 on that.
OK.  That should be better, I hope!  Any other cases?
Attached file Testcase #2
frame: TableCell(th)(0) (0x157cc48) style: 0x157ca98 {}
Wrong parent style context:  style: 0x157c5b0 {}
should be using:  style: 0x1581228 {}
Ah, that case is interesting.

We resolve style on the <table> first.  This recurses down to the table cell, we resolve style on that, discover that it needs a reframe, do NOT set the new style context on it (because we need the old one to properly tear down the frame) and bail out.

Then we process our restyled frames (of which we have two: te table and the th).  The table is processed first, so we mark it as needing reflow, then verify the style tree.  At this point the table cell still has the "wrong" (old) style context.

Maybe we should just verify the style tree starting at the "rootmost" of the frames we process in one processing loop?

In any case, that all doesn't depend on table pseudos; there aren't any table pseudos in this case, in fact.
Blocks: 338703
QA Contact: ian → style-system
This seems to be the last known "Wrong parent style context" bug.  Once it's fixed, I can easily determine whether there are other ways to hit "Wrong parent style context" :)
This is actually a case of the warning being wrong, basically... We're verifying an invariant at a time when there's no reason it should hold.  :(
Blocks: 382204
Attached patch Like soSplinter Review
This is the last case I know of where we get these warnings incorrectly, so I turned them into asserts.  This patch fixes this bug and bug 382204.

The asserts will fire on the testcase in bug 396645, but that's the only case I know of where they will fire.
Assignee: nobody → bzbarsky
Attachment #281549 - Flags: superreview?(dbaron)
Attachment #281549 - Flags: review?(dbaron)
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Summary: "Wrong parent style context" involving table pseudo frames → [FIX]"Wrong parent style context" involving table pseudo frames
Target Milestone: --- → mozilla1.9 M9
Comment on attachment 281549 [details] [diff] [review]
Like so

r+sr+a1.9=dbaron.  Sorry for the delay.
Attachment #281549 - Flags: superreview?(dbaron)
Attachment #281549 - Flags: superreview+
Attachment #281549 - Flags: review?(dbaron)
Attachment #281549 - Flags: review+
Attachment #281549 - Flags: approval1.9+

Would be nice to add the testcases (which would assert if they were still failing, now) to whatever bug 397725 decides on.
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Crashtest checked in.  "Wrong parent style context" is now an assertion, so once assertions turn Tinderbox orange, this will be properly regression-tested.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.