An unknown error while printing SVG file

VERIFIED FIXED in Firefox 18

Status

()

defect
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: virgil.dicu, Assigned: jwatt)

Tracking

({regression, verifyme})

17 Branch
mozilla20
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17 wontfix, firefox18+ verified, firefox19+ verified, firefox20+ verified, firefox-esr17- wontfix, b2g18 fixed)

Details

Attachments

(7 attachments)

Reporter

Description

7 years ago
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

7 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
Perhaps a regression from bug 614723?
Keywords: regression
More likely bug Bug 614732

Updated

7 years ago
Blocks: 614732
Version: unspecified → 17 Branch
Assignee: nobody → jwatt
Assignee

Updated

7 years ago
Assignee

Comment 4

7 years ago
Why are we tracking this? I doubt printing bugs will bubble to the top of my priority queue any time soon.
(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.

Comment 6

7 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.

Comment 7

7 years ago
Trying to print this file will result in an error

Updated

7 years ago
Blocks: 818446
Assignee

Comment 8

7 years ago
I'll dig into this after bug 786894 and bug 806432.
bug 818435 may be related.
(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.
+cc: dholbert for a look...
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
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

7 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

7 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

7 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 20

7 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

7 years ago
roc, any ideas on how to best fix this? (Analysis from comment 14.)
We should not recreate the nsSimplePageSequenceFrame.
Assignee

Updated

7 years ago
Depends on: 822378
Assignee

Comment 23

7 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.
Adding, needinfo for :Virgil & setting the QA contact as :juanb, to help with verfication here once this is ready.
Flags: needinfo?(virgil.dicu)
Keywords: qawanted, verifyme
QA Contact: jbecerra
Assignee

Comment 25

7 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

Updated

7 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)
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

7 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 30

7 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.
https://hg.mozilla.org/mozilla-central/rev/55b179e8c980
https://hg.mozilla.org/mozilla-central/rev/d622ae0a8a23
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Assignee

Comment 32

7 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

7 years ago
Attachment #693197 - Flags: approval-mozilla-beta?
Attachment #693197 - Flags: approval-mozilla-aurora?
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+
Attachment #693197 - Flags: approval-mozilla-beta?
Attachment #693197 - Flags: approval-mozilla-beta+
Attachment #693197 - Flags: approval-mozilla-aurora?
Attachment #693197 - Flags: approval-mozilla-aurora+
Juan,Virgil : can you guys please have explicit test cases/testing around SVG  for FF18 beta 5 , around this bug. Thanks !
Reporter

Comment 37

7 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
Flags: needinfo?(virgil.dicu)
Keywords: qawanted
Assignee

Updated

7 years ago
Blocks: 818435
This is reproducing on 17.0.2 ESR.
Will it be ported on 17ESR?
Reporter

Comment 39

7 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
Verified Fixed on FF 20.b2 Win 7 x64, Ubuntu 12.04 x86 and Mac OS 10.8
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.