334829 - FindFrameWithContent() can't find frame for

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[jag (Peter Annema)]

Danger Inc. has a fix for this which I'll attach shortly. Here's the situation we ran into:

// In a table with a <col> element the table col frame is inside
// tableOuterFrame (but it's a sibling of tableFrame).
//
// In the content tree the <col> element is inside the table element.
//
// So the content tree and frame tree structures don't match, and
// FindFrameWithContent() for the content element after the <col>
// element fails because of the bad hint (the hint is the <col>
// frame, which isn't the prev sibling for the frame sought after).

The bug is that listName (which starts out nsnull) doesn't get set in the path that sets aHint to nsnull, so we exit the inner do/while loop, set aParentFrame to its NifOrSpecialSibling, and try again (with aHint == nsnull this time), while we should retry with the original aParentFrame (with aHint == nsnull this time) first.

If I read it correctly, that's what the old |goto| based code did. See also bug 140275.

Comment 1

[jag (Peter Annema)]

Attachment: Make inner loop test additional flag that gets set when aHint gets cleared.

Patch courtesy of Danger, Inc., with comment changes (merging theirs and the existing one) by me. I think I got the comment right, but feel free to suggest better wording.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This would actually work if columns were honest out-of-flows, due to the

  kidFrame = aFrameManager->GetPlaceholderFrameFor(kidFrame);

code in FindFrameWithContent...

I wonder whether fieldset and legend have similar issues, actually.

Comment 3

[jag (Peter Annema)]

Checked in, 2006-04-21 04:13, layout/base/nsCSSFrameConstructor.cpp, v1.1223

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Regression test

Comment 5

[Boris Zbarsky [:bzbarsky]]

Checked in that test.