478811 - [FIX]IsTableRelated check in GetAbsoluteContainingBlock is incorrect

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Boris Zbarsky [:bzbarsky]]

The IsTableRelated check in GetAbsoluteContainingBlock checks for table-related display types and skips frames that have them.  However, when calling ProcessChildren the push or not of the parent as an absolute containing block depends on the code being used to construct the parent.  This means that we end up with different absolute containing blocks on the two codepaths.  Test that shows this coming up, then a fix.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Reference

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: I think this is the right check

Comment 4

[Bernd]

Comment on attachment 362654 [details] [diff] [review]
I think this is the right check

6999     // being table-related frames are not containers for absolutely
7000     // positioned child frames.
7001     const nsStyleDisplay* disp = frame->GetStyleDisplay();
7002 
7003     if (disp->IsPositioned() && !IsTableRelated(disp->mDisplay, PR_TRUE)) {

we have this code in our tree already??? ;-)

Comment 5

[Bernd]

since 2005 https://bugzilla.mozilla.org/attachment.cgi?id=198046&action=diff

bug 307076

Comment 6

[Boris Zbarsky [:bzbarsky]]

The code you cite is checking mDisplay.  We want to be checking the frame type.

Thanks for the pointer to bug 307076, btw.  I'll make a testcase out of that.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/37405bb5ffd3

Comment 8

[Bernd]

It would be cool if the patch would be against trunk and not a intermediate state of your tree, which I am not aware off, which is also not referenced in this bug. The relevant bug is  478754. I am not sure how a review can be accomplished under those conditions. I though that I review a expansion towards the remaining table types. Which was wrong. Nevertheless the patch is correct.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Er, sorry.  I had in fact forgotten that this applied on top of bug 478754...

Thanks for double-checking the patch, by the way.  This stuff is ... delicate.