Closed
Bug 813531
Opened 12 years ago
Closed 12 years ago
An unknown error while printing SVG file
Categories
(Core :: SVG, defect)
Tracking
()
People
(Reporter: virgil.dicu, Assigned: jwatt)
References
Details
(Keywords: regression, verifyme)
Attachments
(7 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
image/svg+xml
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
roc
:
review+
bajaj
:
approval-mozilla-aurora+
bajaj
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
roc
:
review+
bajaj
:
approval-mozilla-aurora+
bajaj
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121119074111 17.0 ESR
Build ID: 20121119183901 17.0 RC build 2
Steps to reproduce:
1. Open http://upload.wikimedia.org/wikipedia/en/c/ce/SVG-logo.svg
2. Open File>Print or press Ctrl+P to print the logo
3. Press OK in the printer pop-up window
Expected result:
Logo should be printed.
Actual result:
An unknown error appears.
Works for Google Chrome. Works for Firefox 16.0.2.
This error is intermittent. Searching for regression range.
Reporter | ||
Comment 1•12 years ago
|
||
Regression range from mozregression
Last good nightly: 2012-07-20
First bad nightly: 2012-07-21
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3a05d298599e&tochange=446b788ab99d
Comment 3•12 years ago
|
||
More likely bug Bug 614732
Updated•12 years ago
|
tracking-firefox18:
--- → ?
tracking-firefox19:
--- → ?
tracking-firefox20:
--- → ?
tracking-firefox-esr17:
--- → ?
Updated•12 years ago
|
Assignee: nobody → jwatt
Assignee | ||
Updated•12 years ago
|
status-firefox17:
--- → affected
Assignee | ||
Comment 4•12 years ago
|
||
Why are we tracking this? I doubt printing bugs will bubble to the top of my priority queue any time soon.
Comment 5•12 years ago
|
||
(In reply to Jonathan Watt [:jwatt] from comment #4)
> Why are we tracking this? I doubt printing bugs will bubble to the top of my
> priority queue any time soon.
Because it's a recent regression and prevents printing of SVGs.
Updated•12 years ago
|
Because this bug is seriously affecting the main functionality of our website I've been researching it a bit further. These are my conclusions:
1. Bigger svg filesize makes the problem occur more often, above 1mb allmost 100%
2. Printing files fast after another makes the problem occur more often
3. Using a slower/faster pc makes little difference
4. Firefox on Mac OSX 10.8.2 is not affected
I've attached an SVG file which has an allmost 100% error occurrence.
I hope this helps to get it up the queue a bit.
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 8•12 years ago
|
||
I'll dig into this after bug 786894 and bug 806432.
Comment 9•12 years ago
|
||
bug 818435 may be related.
Comment 10•12 years ago
|
||
(In reply to Jonathan Watt [:jwatt] (offline Thu-Sun) from comment #8)
> I'll dig into this after bug 786894 and bug 806432.
Agreed on prioritizing bug 786894 above this (new regression in 18), but I think we should fix this before bug 806432 given user impact.
Comment 11•12 years ago
|
||
+cc: dholbert for a look...
Comment 12•12 years ago
|
||
I tried to Print Preview the URL from comment 0 in my 64-bit Linux nightly & my debug build.
I do see *some* of the content in the print-preview rendering -- the rightmost part of the "flower" logo is almost always visible, and sometimes the "SVG" text is, too.
However:
(a) If I scroll the "printed" page up and down, the SVG content moves on the page, almost as if it's trying to stay in my viewport... except it moves faster than I scroll, so it moves off the bottom of the screen as I scroll downward. (I have to my window shorter than the page, so that there's a scrollbar, in order to test this part.)
(b) In my debug build, I get tons of instances of this warning as I scroll:
WARNING: Outer object paint value used outside SVG glyph: file /scratch/work/builds/mozilla-central/mozilla-central.11-08-17.14-31/mozilla/layout/svg/nsSVGGlyphFrame.cpp, line 1060
Comment 13•12 years ago
|
||
Actually, I see that warning when simply viewing (and clicking around in) the SVG-logo.svg from comment 0 -- so that's not print-specific.
Also: In my linux nightly, I can print mkopinga's attached SVG file, without any error-dialogs or anything, and the printout looks like the browser's rendering -- so that seems to be WORKSFORME.
I can print SVG-logo.svg without any error-dialogs too, but the printout just shows the rightmost tip of the "flower" logo and nothing else, so it's still broken.
Assignee | ||
Comment 14•12 years ago
|
||
Thanks for taking a look at this, Daniel.
I did a bit of my own digging and so far have tracked the issue down to the return of the NS_ERROR_FAILURE error code here:
http://hg.mozilla.org/mozilla-central/annotate/352bd17710c8/layout/generic/nsSimplePageSequence.cpp#l587
This occurs under the following stack:
nsSimplePageSequenceFrame::PrePrintNextPage
nsPrintEngine::PrePrintPage
nsPagePrintTimer::Notify
nsTimerImpl::Fire
I've yet to figure out why mCurrentPageFrame is null at that point.
Assignee | ||
Comment 15•12 years ago
|
||
It seems mCurrentPageFrame is null, not because someone resets it, but rather because the nsSimplePageSequenceFrame is destroyed and recreated without initializing it properly. (What's a bit confusing is that most of the time it gets recreated at the same address, making it look like it's the same nsSimplePageSequenceFrame.)
Assignee | ||
Comment 16•12 years ago
|
||
Okay, what seems to be happening here is that we hit this line (added in the final patch in bug 614732) that adds the nsChangeHint_ReconstructFrame hint to handle stacking context changes:
http://hg.mozilla.org/mozilla-central/annotate/352bd17710c8/content/svg/content/src/nsSVGSVGElement.cpp#l935
under the stack:
nsSVGSVGElement::ChildrenOnlyTransformChanged
nsSVGOuterSVGFrame::NotifyViewportOrTransformChanged
nsSVGOuterSVGFrame::Reflow
nsContainerFrame::ReflowChild
nsCanvasFrame::Reflow
nsContainerFrame::ReflowChild
nsPageContentFrame::Reflow
nsContainerFrame::ReflowChild
nsPageFrame::Reflow
nsContainerFrame::ReflowChild
nsSimplePageSequenceFrame::Reflow
nsContainerFrame::ReflowChild
ViewportFrame::Reflow
PresShell::DoReflow
PresShell::ProcessReflowCommands
PresShell::FlushPendingNotifications
PresShell::FlushPendingNotifications
nsPrintEngine::ReflowPrintObject
nsPrintEngine::ReflowDocList
nsPrintEngine::InitPrintDocConstruction
nsPrintEngine::DoCommonPrint
nsPrintEngine::CommonPrint
nsPrintEngine::Print
nsDocumentViewer::Print
We're hitting this because in nsSVGOuterSVGFrame::Reflow the viewport size is different for printing, resulting in the COORD_CONTEXT_CHANGED bit being added to changeBits, resulting in NotifyViewportOrTransformChanged being called.
Assignee | ||
Comment 17•12 years ago
|
||
Assignee | ||
Comment 18•12 years ago
|
||
Assignee | ||
Comment 19•12 years ago
|
||
Assignee | ||
Comment 20•12 years ago
|
||
I doesn't look like there's a simple way to try and reinitialize things (mCurrentPageFrame) when the nsSimplePageSequenceFrame is recreated.
Assignee | ||
Comment 21•12 years ago
|
||
roc, any ideas on how to best fix this? (Analysis from comment 14.)
We should not recreate the nsSimplePageSequenceFrame.
Assignee | ||
Comment 23•12 years ago
|
||
Right now on the initial nsSVGOuterSVGFrame::Reflow() that method adds COORD_CONTEXT_CHANGED to changeBits, causing it to call NotifyViewportOrTransformChanged. If the outer-<svg> has a viewBox or synthesized viewBox, then NotifyViewportOrTransformChanged converts that to TRANSFORM_CHANGED, causing it to call nsSVGSVGElement::ChildrenOnlyTransformChanged passing the TRANSFORM_CHANGED. Since nsSVGSVGElement::mHasChildrenOnlyTransform is not at that point initialized (is set to false), ChildrenOnlyTransformChanged dispatches a nsChangeHint_ReconstructFrame for the nsSVGOuterSVGFrame, causing us to reconstruct the entire frame tree.
This patch prevents us from calling NotifyViewportOrTransformChanged on the initial reflow, and has us just set nsSVGSVGElement::mHasChildrenOnlyTransform directly instead (the current code for setting that in nsSVGOuterSVGFrame::Reflow is bogus and doesn't actually do anything). Unfortunately, the undesirable frame tree reconstruction that was occurring under NotifyViewportOrTransformChanged has been hiding pre-existing bugs, so the patch results in various test failures by exposing them. One of those bugs is bug 822378 (patch landed).
The remaining test failures I'm getting with this patch are:
reftests/svg/as-image/img-foreignObject-1.html
reftests/svg/as-image/img-foreignObject-embed-1.html
reftests/svg/as-image/img-foreignObject-iframe-1a.html
reftests/svg/as-image/img-foreignObject-iframe-1b.html
Still working on sorting out what the issue is there.
Comment 24•12 years ago
|
||
Adding, needinfo for :Virgil & setting the QA contact as :juanb, to help with verfication here once this is ready.
Assignee | ||
Comment 25•12 years ago
|
||
(In reply to Jonathan Watt [:jwatt] from comment #23)
> Still working on sorting out what the issue is there.
So here's what's going on...there are two preexisting bugs that are masked by the reconstruct-the-frame-tree bug. Both manifest when the SVG is first reflowed for a zero sized viewport, and then later reflowed for a non-zero sized viewport. This is happening for the four foreignObject tests listed in comment 23 because SVGDocumentWrapper::OnStopRequest calls FlushLayout(), forcing reflow of the nsSVGOuterSVGFrame before its container document has had its initial reflow and set the dimensions of the frame for the embedding <img>. When the embedding frame for the <img> gets its first reflow, the nsSVGOuterSVGFrame is then reflowed at a non-zero size.
The main pre-existing bugs is that when we reflow with a zero-sized viewport, we end up giving the direct children frames of the nsSVGOuterSVGFrame empty overflow areas. This is because FinishAndStoreOverflow takes account of the children-only transform on them, and that transform ends up being the identity matrix because that's what's returned by nsSVGSVGElement::GetViewBoxTransform() (under IsSVGTransformed) when viewportWidth/viewportHeight is zero. The bug is that we don't have any logic to make sure that we update these overflow areas when we reflow the nsSVGOuterSVGFrame with a valid viewport size.
The other pre-existing bug is that in nsSVGOuterSVGFrame::NotifyViewportOrTransformChanged we discard the COORD_CONTEXT_CHANGED flag for synthetic viewBoxes. This is wrong because in the case of a synthetic viewBox the viewBox rect changes as the viewport changes (unlike an explicit viewBox's rect).
Assignee | ||
Comment 26•12 years ago
|
||
Attachment #693197 -
Flags: review?(roc)
Assignee | ||
Updated•12 years ago
|
Attachment #693056 -
Attachment description: WIP patch to stop reconstructing the frame tree on the initial reflow → patch to stop reconstructing the frame tree on the initial reflow
Attachment #693056 -
Flags: review?(roc)
Attachment #693056 -
Flags: review?(roc) → review+
Comment on attachment 693197 [details] [diff] [review]
patch to fix the two pre-existing bugs explained in comment 25
Review of attachment 693197 [details] [diff] [review]:
-----------------------------------------------------------------
Got tests for these?
::: layout/svg/nsSVGOuterSVGFrame.cpp
@@ +440,5 @@
> + anonChild->AddStateBits(NS_FRAME_IS_DIRTY);
> + for (nsIFrame* child = anonChild->GetFirstPrincipalChild(); child;
> + child = child->GetNextSibling()) {
> + child->AddStateBits(NS_FRAME_IS_DIRTY);
> + }
You don't need this loop. NS_FRAME_IS_DIRTY forces all descendants to be reflowed too.
Attachment #693197 -
Flags: review?(roc) → review+
Assignee | ||
Comment 28•12 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #27)
> Got tests for these?
The four tests mentioned in comment 23.
> You don't need this loop. NS_FRAME_IS_DIRTY forces all descendants to be
> reflowed too.
Actually ReflowSVG does not treat NS_FRAME_IS_DIRTY that way, so I do. Fixing up how SVG treats the dirty bits is on my list of things to do one day.
Assignee | ||
Comment 29•12 years ago
|
||
Assignee | ||
Comment 30•12 years ago
|
||
The current status after these two patches is that SVG files can now print without being blocked by the "An unknown error occurred while printing" error message. That said, not all SVG files are quite printing correctly. The attachment "This file won't print" now prints fine, but the SVG-logo.svg example from comment 0 seems to be printing with the wrong offset. I'm not sure yet at which point between the landing of bug 614732 and now that offset was broken since nightly builds in that time range can't print, of course. It's possible that bug 614732 broke the offset too.
Anyway, since the original blocking error message is now fixed I'm not sure whether I should leave this open for the offset bug, or open a different bug for that.
Comment 31•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/55b179e8c980
https://hg.mozilla.org/mozilla-central/rev/d622ae0a8a23
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Assignee | ||
Comment 32•12 years ago
|
||
Comment on attachment 693056 [details] [diff] [review]
patch to stop reconstructing the frame tree on the initial reflow
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 614732
User impact if declined: SVG with a viewBox (fairly common) won't print
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): low enough risk for beta 5 I think
String or UUID changes made by this patch: none
Attachment #693056 -
Flags: approval-mozilla-beta?
Attachment #693056 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•12 years ago
|
Attachment #693197 -
Flags: approval-mozilla-beta?
Attachment #693197 -
Flags: approval-mozilla-aurora?
Comment 33•12 years ago
|
||
Comment on attachment 693056 [details] [diff] [review]
patch to stop reconstructing the frame tree on the initial reflow
Approving on branches.Confirmed with :jwatt about the risk being low.In addition, we will request QA to do a lot of SVG testing for beta 5
Attachment #693056 -
Flags: approval-mozilla-beta?
Attachment #693056 -
Flags: approval-mozilla-beta+
Attachment #693056 -
Flags: approval-mozilla-aurora?
Attachment #693056 -
Flags: approval-mozilla-aurora+
Updated•12 years ago
|
Attachment #693197 -
Flags: approval-mozilla-beta?
Attachment #693197 -
Flags: approval-mozilla-beta+
Attachment #693197 -
Flags: approval-mozilla-aurora?
Attachment #693197 -
Flags: approval-mozilla-aurora+
Comment 34•12 years ago
|
||
Juan,Virgil : can you guys please have explicit test cases/testing around SVG for FF18 beta 5 , around this bug. Thanks !
Assignee | ||
Comment 35•12 years ago
|
||
Comment 36•12 years ago
|
||
Reporter | ||
Comment 37•12 years ago
|
||
Mozilla/5.0 (X11; Linux i686; rv:18.0) Gecko/20100101 Firefox/18.0
Marking as verified in beta 5, the initial issue is no longer reproducible. Print preview/landascape/portrait - all print correctly. Verified with Ubuntu 12.04, Windows 7, Mac OS 10.7.
Also ran related SVG test cases (listed in the Firefox 18b5 Test Plan) - all looks good.
https://wiki.mozilla.org/Releases/Firefox_18/Test_Plan#SVG_Testing
Comment 38•12 years ago
|
||
This is reproducing on 17.0.2 ESR.
Will it be ported on 17ESR?
Reporter | ||
Comment 39•12 years ago
|
||
Verified the fix in Firefox 19.0 Beta 2 - no longer reproducible on Windows 7, Mac OS X 10.7.5 and Ubuntu 12.04.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130116072953
Updated•12 years ago
|
Comment 40•12 years ago
|
||
Verified Fixed on FF 20.b2 Win 7 x64, Ubuntu 12.04 x86 and Mac OS 10.8
Updated•12 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•