Closed
Bug 1246883
Opened 9 years ago
Closed 7 years ago
Regression: SVG prints without page margins and without header/footer lines
Categories
(Core :: Printing: Output, defect)
Tracking
()
VERIFIED
FIXED
mozilla59
People
(Reporter: birger.retterstol.olaisen, Assigned: mantaroh)
References
Details
(Keywords: regression)
Attachments
(5 files)
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.
Reporter | ||
Updated•9 years ago
|
Component: Untriaged → General
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
Could you attach the svg file to the bug report, please.
Flags: needinfo?(birger.retterstol.olaisen)
Reporter | ||
Comment 2•9 years ago
|
||
Flags: needinfo?(birger.retterstol.olaisen)
Reporter | ||
Comment 3•9 years ago
|
||
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)
Comment 8•9 years ago
|
||
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)
Comment 9•8 years ago
|
||
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
Comment 10•8 years ago
|
||
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.
Assignee | ||
Comment 11•7 years ago
|
||
(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
Assignee | ||
Comment 12•7 years ago
|
||
(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 hidden (mozreview-request) |
Comment 14•7 years ago
|
||
mozreview-review |
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+
Comment 15•7 years ago
|
||
(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 hidden (mozreview-request) |
Assignee | ||
Comment 17•7 years ago
|
||
mozreview-review-reply |
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.
Assignee | ||
Comment 18•7 years ago
|
||
Assignee | ||
Comment 19•7 years ago
|
||
(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)
Comment 20•7 years ago
|
||
Pushed by mantaroh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/727a90ad06e7
Load UA Stylesheet when printing the SVG document. r=jwatt
Comment 21•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 22•7 years ago
|
||
This grafts cleanly to Beta and looks pretty safe. Please nominate it for Beta approval.
status-firefox57:
--- → wontfix
status-firefox58:
--- → affected
status-firefox-esr52:
--- → wontfix
Flags: needinfo?(mantaroh)
Assignee | ||
Comment 23•7 years ago
|
||
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?
Comment 24•7 years ago
|
||
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)
Updated•7 years ago
|
Flags: qe-verify- → qe-verify+
Comment 25•7 years ago
|
||
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.
Comment 26•7 years ago
|
||
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+
Comment 27•7 years ago
|
||
bugherder uplift |
Comment 28•7 years ago
|
||
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.
Description
•