767593 - "Already have an undisplayed context entry for aContent" when printing

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase (print me)

Printing the testcase (e.g. to PDF) triggers:

###!!! ASSERTION: Already have an undisplayed context entry for aContent: '!GetUndisplayedContent(aContent)', file layout/base/nsFrameManager.cpp, line 338

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Jesse Ruderman]

Attachment: testcase 2 (print me)

This one triggers an additional assertion:

###!!! ASSERTION: node in map twice: 'Not Reached', file layout/base/nsFrameManager.cpp, line 1911

Comment 3

[Kang-Hao (Kenny) Lu [:kennyluck]]

Attachment: tweak the assertion and fix a bug

The issue is that replicated fixed frames, which are only generated in paginated mode, might contain undisplayed content. Also, the fact that attachment 635934 [details] doesn't generate the warning attachment 635956 [details] generates is due to a slight flaw in nsFrameManagerBase::UndisplayedMap::AppendNodeFor, which is fixed in this patch.

It might or might not be a clearer solution just to remove all the use of mUndisplayedItems/mUndisplayedMap map in pagination mode though perhaps that should be a followup bug. However, I am not sure whether eContext_PageLayout needs it.

I also bumped into bug 485893. I might talk a look into that later.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Wouldn't it be better to skip calls to SetUndisplayedContent when aState.mCreatingExtraFrames?

Comment 5

[Jesse Ruderman]

In the patch description, "dumplicated" is either a typo or a sad relationship status.

Comment 6

[Kang-Hao (Kenny) Lu [:kennyluck]]

(In reply to Jesse Ruderman from comment #5)
> In the patch description, "dumplicated" is either a typo or a sad
> relationship status.

It's "duplicated" :p

I plan to do study a bit more and do Comment 4, but I probably wouldn't have the chance to do that by the weekend.

Comment 7

[Mats Palmgren (inactive)]

Attachment: fix+tests

Is this is what you suggested in comment 4?

https://tbpl.mozilla.org/?tree=Try&rev=b6f73670c85f
https://tbpl.mozilla.org/?tree=Try&rev=c0c592796ab7

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8456833 [details] [diff] [review]
fix+tests

Yep, that works.  Thanks!

Comment 9

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e280e3ce940b

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e280e3ce940b