300674 - Crash with Print Preview with this testcase [@ 0x00000009 - nsCSSFrameConstructor::ReinsertContent]

Status: RESOLVED WORKSFORME

(Core :: Print Preview, defect)

Description

[Martijn Wargers (dead)]

This is a follow-up from bug 299107.
The testcase I'll attach shows that it is still possible to crash Mozilla on
print preview in a way that is similar to bug 299107

Comment 1

[Martijn Wargers (dead)]

Attachment: popup needed for testcase

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase

Follow the steps in the testcase to trigger the crash

Comment 3

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b3) Gecko/20050711 SeaMonkey/1.0a
Talkback TB7466175H

Comment 4

[Hermann Schwab]

Attachment: TB7466175 Talkback report

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yeah, the problem here is that the DOM we're print previewing gets modified.  We
need to prevent that from happening is all.

Comment 6

[chris hofmann]

bz, any thoughts on next steps for this one?

Comment 7

[Boris Zbarsky [:bzbarsky]]

We rewrite print preview to not suck?  :(

Seriously, short of cloning the DOM for print preview or modifying various code to deal with print preview's little idiosyncrasies, I'm not sure what we can do.  :(

Comment 8

[Ronald Tilby]

Testcase does not crash for me using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060621 Minefield/3.0a1 ID:2006062104 [cairo]

Comment 9

[Martijn Wargers (dead)]

Yeah, I also don't crash anymore with current trunk build. However the iframe disappears suddenly in print preview, which it shouldn't do.
You shouldn't be able to modify the dom during print preview and that is still possible.
I could imagine it would still be possible to create a similar testcase that still crashes.

Comment 10

[Carsten Book [:Tomcat]]

also no crash for me in Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a6pre) Gecko/20070613 Minefield/3.0a6pre ID:2007061304 [cairo]

Martijn, do you see still this crash ?

Comment 11

[Martijn Wargers (dead)]

Yeahu, worksforme too.
I'll mark this bug worksforme, then.
I kept this open for comment 5, that said "the DOM we're print previewing gets modified. We need to prevent that from happening is all."
But according to roc you should be able to modify the dom on print preview.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Er... we should?  Why, exactly?  I don't buy that.

Modifying it _before_ print preview is fine, but not while print preview is up, imo.

Comment 13

[Eli Friedman]

(In reply to comment #12)
> Modifying it _before_ print preview is fine, but not while print preview is up,
> imo.

There's two issues here: one is that print preview shouldn't be dynamically modifiable, and the second is that we shouldn't crash when dynamic changes happen in paginated contexts.

For the first issue, there's really only one way to fix that correctly, and that's to clone the document before print preview.  Unfortunately, we can't clone HTML documents at the moment, so it's kinda difficult to fix.  I think there's another bug on that, but I don't know the number off the top of my head.

The other issue is that we should be able to handle dynamic changes in paginated contexts without crashing.  At the moment, we can generally handle that smoothly, although I think there might be some issues with tables.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What Eli said... what I meant when I talked to Martijn is that we should be able to tolerate DOM changes in paginated contexts, so DOM changes in print preview shouldn't crash the browser. That does not mean we should allow those changes...