Closed Bug 1246883 Opened 4 years ago Closed 2 years ago

Regression: SVG prints without page margins and without header/footer lines

Categories

(Core :: Printing: Output, defect)

32 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla59
Tracking Status
firefox-esr52 --- wontfix
firefox57 --- wontfix
firefox58 --- verified
firefox59 --- verified

People

(Reporter: birger.retterstol.olaisen, Assigned: mantaroh)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(5 files)

Attached file Kusmi Darjeeling.pdf
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.103 Safari/537.36

Steps to reproduce:

Tried to print a SVG file with image positioned at x=0 y=0.
The margins on the Page Setup dialog box were at default values, all 12.7mm.


Actual results:

The SVG image was positioned right at the top and left edge of the page.
The header was printed on top of the topmost images.


Expected results:

The SVG image should have been placed with a margin of 12.7mm all around.
Component: Untriaged → General
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
Component: General → Printing: Output
Product: Firefox → Core
Could you attach the svg file to the bug report, please.
Flags: needinfo?(birger.retterstol.olaisen)
Attached image Kusmi_Darjeeling.svg
Flags: needinfo?(birger.retterstol.olaisen)
Attached image Kusmi_Darjeeling.png
Attached image 1246883.svg
Print preview has the same issue too.

Reg range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e86a0d92d174&tochange=cbe4f69c2e9c

I thnk it's the result of one these bug reports:
Jonathan Watt — Bug 1011806 - Stop loading user-agent and user style sheets for SVG-as-an-image (the main UA sheets svg.css, html.css, etc. will still load on demand). r=bz CLOSED TREE
	7dca8bddf997	Jonathan Watt — No bug - Add a comment to nsStyleSet::SizeOfIncludingThis noting that it does _not_ count the size of the sheets in mSheets, only the size of the mSheets buffer.
	feff69cd698f	Jonathan Watt — Bug 1013936, part 2 - Only load the html.css UA style sheet on-demand for SVG documents. r=bz CLOSED TREE
	027f3c05a629	Jonathan Watt — Bug 1013936, part 1 - Add various methods to nsIDocument to help determine its type. r=bz
	b42ec325e34d	Jonathan Watt — Bug 1008455 - Avoid loading the xul.css UA style sheet when possible. r=bz CLOSED TREE
	f11fd47e89a7	Jonathan Watt — Bug 1015147 - Use the style sheet cache to store the user-agent style sheets svg.css and mathml.css so that we don't create new instances for each new document. r=bz
	bca61e86b826	Jonathan Watt — Bug 1014891 - Remove support for XML's 'catalog' style sheets, and provide an internal alternative for the abusing callers of EnsureCatalogStyleSheet. r=bz
Keywords: regression
Version: 44 Branch → 32 Branch
Jonathan, any idea why the SVG style-sheet overrides the settings of the print setup?
Flags: needinfo?(jwatt)
I don't really know anything about printing, but it looks like nsPrintEngine::ReflowPrintObject does some styleSet stuff, and I guess that interacts badly with the nsDocumentViewer::CreateStyleSet changes in https://hg.mozilla.org/mozilla-central/rev/feff69cd698f
Flags: needinfo?(jwatt)
This seems to be due to SVG not getting the following style rules from ua.css:

*|*::-moz-page-sequence, *|*::-moz-scrolled-page-sequence {
  /* Collection of pages in print/print preview. Visual styles may only appear
   * in print preview. */
  display: block !important;
  background: linear-gradient(#606060, #8a8a8a) fixed;
  height: 100%;
}

*|*::-moz-page {
  /* Individual page in print/print preview. Visual styles may only appear
   * in print preview. */
  display: block !important;
  background: white;
  box-shadow: 5px 5px 8px #202020;
  margin: 0.125in 0.25in;
}

*|*::-moz-pagecontent {
  display: block !important;
  margin: auto;
}

*|*::-moz-pagebreak {
  display: block !important;
}

The easiest way to fix this would be to move this line to be after the if-else block:

https://hg.mozilla.org/mozilla-central/annotate/678cd6e8be91/layout/base/nsDocumentViewer.cpp#l2331

but that will regress SVG-as-an-image memory use and load time. Better would be to split up ua.css into SVG and non-SVG files in some way, but it's not clear what should apply in the SVG case. A fair bit of digging will be required to figure out which bits [can] apply to the anonymous frames that may be created above the nsSVGOuterSVGFrame.
Assignee: nobody → jwatt
Blocks: 1302489
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Printing SVG file disregards margins entered in the Page Setup dialog box → Regression: SVG prints without page margins and without header/footer lines
I guess another way to fix this would be to use EnsureOnDemandBuiltInUASheet to load that sheet when we print, if there is an appropriate place to hook that into.
(In reply to Jonathan Watt [:jwatt] from comment #10)
> I guess another way to fix this would be to use EnsureOnDemandBuiltInUASheet
> to load that sheet when we print, if there is an appropriate place to hook
> that into.

I talked with jwatt on IRC, I'll take this bug.

I think that it's better to add the code of loading ua.css when reflow of nsPrintEngine[1].

[1] https://hg.mozilla.org/try/rev/8a886ef96158ae5a08530ae09200d770938b6d0b
Assignee: jwatt → mantaroh
Status: NEW → ASSIGNED
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #11)
> (In reply to Jonathan Watt [:jwatt] from comment #10)
> > I guess another way to fix this would be to use EnsureOnDemandBuiltInUASheet
> > to load that sheet when we print, if there is an appropriate place to hook
> > that into.
> 
> I talked with jwatt on IRC, I'll take this bug.
> 
> I think that it's better to add the code of loading ua.css when reflow of
> nsPrintEngine[1].
> 
> [1] https://hg.mozilla.org/try/rev/8a886ef96158ae5a08530ae09200d770938b6d0b

Oops, If we use the 'EnsureOnDemandBuiltInUASheet', Gecko will crash.
In this step, we don't call EndUpdate yet, thus I think that we should use 'PrependStyleSheet'.
Comment on attachment 8905736 [details]
Bug 1246883 - Load UA Stylesheet when printing the SVG document.

https://reviewboard.mozilla.org/r/177126/#review209004

Sorry I dropped the ball on this one. I meant to dig into whether there is a better way to do this, but apparently I'm never going to carve out the time to do that. For now, this seems sane.

::: layout/printing/nsPrintEngine.cpp:2306
(Diff revision 1)
>    rv = aPO->mViewManager->Init(printData->mPrintDC);
>    NS_ENSURE_SUCCESS(rv,rv);
>  
>    StyleSetHandle styleSet = mDocViewerPrint->CreateStyleSet(aPO->mDocument);
>  
> +  // The SVG document only loads minimal-xul.css, so it doesn't apply other

Please put the comment inside the |if| block.
Attachment #8905736 - Flags: review?(jwatt) → review+
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #12)
> Oops, If we use the 'EnsureOnDemandBuiltInUASheet', Gecko will crash.
> In this step, we don't call EndUpdate yet, thus I think that we should use
> 'PrependStyleSheet'.

Can you add a stack trace and description of what goes wrong (as best you can make out) to this bug to make sure this is documented for future bugzilla archeologists.
Flags: needinfo?(mantaroh)
Comment on attachment 8905736 [details]
Bug 1246883 - Load UA Stylesheet when printing the SVG document.

https://reviewboard.mozilla.org/r/177126/#review209004

Thanks. Sorry for my late reply.
I fixed the comment.
(In reply to Jonathan Watt [:jwatt] (needinfo? me) from comment #15)
> (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #12)
> > Oops, If we use the 'EnsureOnDemandBuiltInUASheet', Gecko will crash.
> > In this step, we don't call EndUpdate yet, thus I think that we should use
> > 'PrependStyleSheet'.
> 
> Can you add a stack trace and description of what goes wrong (as best you
> can make out) to this bug to make sure this is documented for future
> bugzilla archeologists.

EnsureOnDemandBuildInUASheet() calls BeginUpdate()[1]. In nsPrintEngine::ReflowPrintObject, gecko create the style set which already opened.[2] So we don't need to create style set again.

[1] https://searchfox.org/mozilla-central/rev/9d920555ec81f1c9d4b7fa3b08e23eb88efb60e1/layout/base/nsDocumentViewer.cpp#2339
[2] https://searchfox.org/mozilla-central/rev/9d920555ec81f1c9d4b7fa3b08e23eb88efb60e1/layout/printing/nsPrintEngine.cpp#2280
Flags: needinfo?(mantaroh)
Pushed by mantaroh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/727a90ad06e7
Load UA Stylesheet when printing the SVG document. r=jwatt
https://hg.mozilla.org/mozilla-central/rev/727a90ad06e7
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
This grafts cleanly to Beta and looks pretty safe. Please nominate it for Beta approval.
Flags: needinfo?(mantaroh)
Comment on attachment 8905736 [details]
Bug 1246883 - Load UA Stylesheet when printing the SVG document.

I'd like to uplift this fix to beta.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1013936.
[User impact if declined]: If user uses printing of SVG, Firefox doesn't print the header/footer and related strings. 
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: Yes, comment #0. (We need to use attachment 8717389 [details])
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: no. this is very limited modification.
[String changes made/needed]: n/a
Flags: needinfo?(mantaroh)
Attachment #8905736 - Flags: approval-mozilla-beta?
Hi Florin,
could you help find someone to verify if this issue was fixed as expected on the latest Nightly build? Thanks!
Flags: qe-verify-
Flags: needinfo?(florin.mezei)
Flags: qe-verify- → qe-verify+
Reproduced the bug on an older build Nightly 59.0a1, from 12.12.2017 on Windows 7 x 64.

Confirm the fix with the latest Nightly 59.0a1, build ID 20180109100117 on Windows 10 x64, Windows 7 x64, Ubuntu 16.04 and Mac OS X 10.12.
Status: RESOLVED → VERIFIED
Flags: needinfo?(florin.mezei)
Comment on attachment 8905736 [details]
Bug 1246883 - Load UA Stylesheet when printing the SVG document.

Fix an SVG printing regression and was verified. Beta58+.
Attachment #8905736 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed using Firefox 58 beta 16 under Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.