If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

"ASSERTION: Ancestors of nodes with frames to be constructed lazily should have frames and not have NEEDS_FRAME bit set" with XBL, frameset

RESOLVED FIXED

Status

()

Core
Layout: HTML Frames
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Jesse Ruderman, Assigned: tnikkel)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
x86
Mac OS X
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
Created attachment 440124 [details]
testcase

###!!! ASSERTION: Ancestors of nodes with frames to be constructed lazily should have frames and not have NEEDS_FRAME bit set: 'content->GetPrimaryFrame() && !content->HasFlag(NODE_NEEDS_FRAME)', file /Users/jruderman/mozilla-central/layout/base/nsCSSFrameConstructor.cpp, line 6198

This assertion was added in rev f80de1a7c604:
user:        Timothy Nikkel
date:        Mon Jan 18 03:26:40 2010 -0600
summary:     Bug 502937. Part 3. Implement lazy frame construction. r=bzbarsky sr=roc
(Assignee)

Comment 1

8 years ago
So we have

<frameset style="-moz-binding: url(#foo)">
  <frame id="y"></frame>
</frameset>

The foo binding has

    <content>
      <frame xmlns="http://www.w3.org/1999/xhtml">
        <children xmlns="http://www.mozilla.org/xbl"/>
      </frame>
    </content>

We end up with a flattened tree that looks like

<frameset style="-moz-binding: url(#foo)">
  <frame>
    <frame id="y"></frame>
  </frame>
</frameset>

but the frameset frame processes it's own children when constructing frames and it just looks at its principal child list (no ChildIterator), so it ignores the anonymous <frame> element. Then we insert a span as a child of the <frame id="y"> and we walk up through the anonymous <frame> in nsCSSFrameConstructor::MaybeConstructLazily, and it does not have a frame, so we assert.

We could add some conditions to the assert about the parent of the current content being a leaf frame with the reasoning that leaf frames construct their own children and may do something unusual. Or we could change nsFramesetFrame::Init to use ChildIterator. Or maybe there is a better solution. What do you think Boris?
Using ChildIterator in the frameset code seems like the good thing to do long-term in terms of general sanity, as long as there are no assumptions about parent chains made in frameset frames...

But changing the assert seems like the safe thing to do if we're worried about the ChildIterator approach.  How worried are we?

I'm not sure there's a better solution here, fwiw.
(Assignee)

Comment 3

8 years ago
Created attachment 441136 [details] [diff] [review]
patch

I think I'm just going to go for the quick, safe fix here and modify the assert.
Assignee: nobody → tnikkel
Attachment #441136 - Flags: review?(bzbarsky)
Comment on attachment 441136 [details] [diff] [review]
patch

r=bzbarsky
Attachment #441136 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 5

8 years ago
http://hg.mozilla.org/mozilla-central/rev/46d0398b5a61
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
(Reporter)

Updated

8 years ago
Depends on: 564063
You need to log in before you can comment on or make changes to this bug.