Closed Bug 959311 Opened 6 years ago Closed 6 years ago

"Open PDF in Preview" for Bootstrap site crashes on OS X

Categories

(Core :: Layout: Form Controls, defect, P3, critical)

26 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: mozilla, Assigned: mats)

References

()

Details

(Keywords: crash, reproducible)

Attachments

(4 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.63 Safari/537.36

Steps to reproduce:

1. Open http://getbootstrap.com/css/
2. File->Print...
3. In the dialog, open the "PDF" menu and choose "Open PDF in Preview"


Actual results:

Firefox 26.0 crashes every time. Confirmed on a co-worker's machine.


Expected results:

Preview.app should have opened and displayed the print preview of the webpage.
Tested on OS X 10.7.5 and 10.8.5
I can definitely reproduce this.

In nsCSSFrameConstructor::CreateContinuingFrame we're hitting NS_RUNTIMEABORT("unexpected frame type"), and of course aborting.  

The caller is nsBlockFrame::ReflowBlockFrame, like so:

3174              bool madeContinuation =
3175                CreateContinuationFor(aState, nullptr, frame);

and "frame" is an nsComboboxControlFrame, which should never ever end up not fully complete, I'd think.
Status: UNCONFIRMED → NEW
Component: General → Layout: Form Controls
Ever confirmed: true
Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8156df33b757&tochange=1d6fe70c79c5 (so all the way back to Firefox 22).

Presumably this is fallout from bug 852501.  Before that we used to assert and return an error and a null continuing frame but not outright abort...  Not sure what the caller did with that error return, exactly.

It looks like nsComboboxControlFrame is just doing nsBlockFrame::Reflow to set aStatus.  And it's passing what can be a constrained-height reflow state there.  All of that seems fairly bogus.  :(

Mats, do you have the bandwidth to look at this?
Blocks: 852501
Flags: needinfo?(matspal)
And Chris, thank you for the bug report!
Thanks for the amazingly fast response time, Boris!
I can't reproduce the crash in FF26, FF27(beta) or a local trunk debug build on OSX 10.9.1

I get a few warnings and one assertion during Print-Preview layout in the debug build:

[56767] WARNING: NS_ENSURE_TRUE(GetOwner()) failed: file image/src/imgRequestProxy.cpp, line 678
[56767] WARNING: Out-of-flow frame got reflowed before its placeholder: file layout/generic/nsPlaceholderFrame.cpp, line 126
[56767] ###!!! ASSERTION: no page sequence frame: 'nullptr != pageSequence', file layout/printing/nsPrintEngine.cpp, line 2458
Severity: normal → critical
Flags: needinfo?(matspal)
Priority: -- → P3
If anyone can reproduce this crash in a debug build, I would be interested in knowing
what the value of 'aStatus' is (after the nsBlockFrame::Reflow call) and a frame tree
dump.
This is quite reproducible for me in a debug build.

aStatus is 19.  I'll attach the frame tree.  Note that the attachment shows the address of the combobox frame we're trying to split too.
Attached file Zipped-up frame dump
The uncompressed dump is about 4MB.  :(
Flags: needinfo?(matspal)
Attached file formatted frame tree
So aStatus is:
#define NS_FRAME_NOT_COMPLETE         0x1
#define NS_FRAME_REFLOW_NEXTINFLOW    0x2
#define NS_FRAME_TRUNCATED         0x0010

This attachment is your frame dump without the Page frames preceding
the page that the crashing combobox frame is on.
I don't see anything unusual about this frame tree.

I wonder why nsBlockFrame returned the above aStatus.  Print-Previewing any
<select size=1> that falls on a page boundary doesn't trigger the assertion
so there must be some special situation here.
Flags: needinfo?(matspal)
Attached file Somewhat self-contained testcase (obsolete) —
This reliably crashes for me.  The CSS is unminified from the site; the HTML is pretty minimal.  The height of the div was picked to put the select on the page boundary.

Still need to work on minimizing the CSS more...
Attached file Minimal-ish testcase
Seems to need all three ingredients: block-level select, constrainted-height select, on page boundary.
Attachment #8364470 - Attachment is obsolete: true
Flags: needinfo?(matspal)
Attached patch fix+testsSplinter Review
I don't think it makes sense to split nsComboboxControlFrames
so we should just always return COMPLETE reflow status for them.
This fixes the crash.

Additionally, I think they should have page-break-inside:avoid
by default (which avoids the crash independently of the fix
above).  I'm not making it !important though, so we want both
changes.

https://tbpl.mozilla.org/?tree=Try&rev=ccd45d275915
Assignee: nobody → matspal
Attachment #8366734 - Flags: review?(bzbarsky)
Flags: needinfo?(matspal)
Comment on attachment 8366734 [details] [diff] [review]
fix+tests

r=me
Attachment #8366734 - Flags: review?(bzbarsky) → review+
And thank you!
https://hg.mozilla.org/integration/mozilla-inbound/rev/044deda0cbcb
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/044deda0cbcb
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.