Closed
Bug 1326418
Opened 7 years ago
Closed 7 years ago
Assertion failure: !mHasActivePage (Missing EndPage() call) when trying to print-to-file http://www.yle.fi/
Categories
(Core :: Printing: Output, defect)
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: smaug, Unassigned)
References
Details
Attachments
(1 file)
2.35 KB,
patch
|
jwatt
:
review-
|
Details | Diff | Splinter Review |
I was trying to test a printing related patch, but even without my patch printing crashes on debug builds / linux, so couldn't really test my patch. Testcase is to load http://www.yle.fi/ and print it to a pdf. (I guess I could test my patch on top of code before bug 1280324 landed)
Reporter | ||
Updated•7 years ago
|
Flags: needinfo?(jwatt)
Comment 1•7 years ago
|
||
What platform? This assertion doesn't happen for me on macOS.
Flags: needinfo?(jwatt)
Comment 2•7 years ago
|
||
(In reply to Jonathan Watt [:jwatt] from comment #1) > What platform? This assertion doesn't happen for me on macOS. smaug is testing on Linux. I've possibly seen something similar on Windows, but don't have time to test now.
Flags: needinfo?(jwatt)
Comment 3•7 years ago
|
||
I was investigating a different printing issue that hit this assertion so I did a quick fix to get around it... The error is that PrintTargetPDF overrides PrintTarget::EndPage so the corresponding DEBUG code there is never invoked, so at the next BeginPage() call (not overridden), the mHasActivePage is still true. The logic around the Begin/EndPage calls in nsSimplePageSequenceFrame also looked wrong to me so I fixed those too.
Pushed by jwatt@jwatt.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/3f8218f821e3 part 1 - Make PrintTargetPDF::EndPage call PrintTarget::EndPage to fix MOZ_ASSERT failures. r=jwatt
Comment 5•7 years ago
|
||
r+ on the PrintTargetPDF changes, which I've landed on your behalf to get that fix into the hands of people like smaug. I'd like to look more closely at the nsSimplePageSequenceFrame changes though.
Flags: needinfo?(jwatt)
Comment 6•7 years ago
|
||
Comment on attachment 8822877 [details] [diff] [review] wip Review of attachment 8822877 [details] [diff] [review]: ----------------------------------------------------------------- As far as I can see the current nsSimplePageSequenceFrame.cpp code is correct and your changes would break things. That said this code is pretty poorly structured. ::: layout/generic/nsSimplePageSequenceFrame.cpp @@ +748,2 @@ > } else { > mCalledBeginPage = false; The point of this code is that after this point mCalledBeginPage should be false so that if we loop around to print another page's worth of selection we will call BeginPage again (mandatory). This change would break that. @@ +780,2 @@ > PR_PL(("***************** End Page (PrintNextPage) *****************\n")); > rv = dc->EndPage(); We do not want to call EndPage unless we know we're looping around and will be calling BeginPage again. (The normal place for EndPage to be called is in DoEndPage.)
Attachment #8822877 -
Flags: review-
Comment 7•7 years ago
|
||
(In reply to Jonathan Watt [:jwatt] from comment #6) > That said this code is pretty poorly structured. I'll put up a patch in bug 1328275 to try and tidy this code up somewhat.
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3f8218f821e3
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•