Closed Bug 1448507 Opened 2 years ago Closed Last year

WebExtension tabs.insertCSS not applied when printing

Categories

(Core :: Printing: Output, defect, P2)

58 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: markp.mckinney, Assigned: jwatt)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.162 Safari/537.36

Steps to reproduce:

Insert CSS into a tab using browser.tabs.insertCSS that affects the current page and use the print function to save as PDF or print.


Actual results:

The CSS that was inserted is not applied in the print. It is applied on the page itself though.


Expected results:

The CSS should have been applied in the print.
Component: Untriaged → WebExtensions: General
Product: Firefox → Toolkit
Component: WebExtensions: General → Printing: Output
Product: Toolkit → Core
Confirming the issue. More context:

1. using "media emulate print" and a |@media print {#mytestelement {display: none}}| rule, I confirmed the stylesheets are applied for print media *outside* of printing
2. but when the same document is printed, the stylesheet is not applied and the test element is printed...

Just for the record, this is severe bug for WebExtension implementors since whatever markup we inject in the page and that we design to be invisible on print does appear on print... This affects all webextension inserting a private bar, notifications, action buttons, etc to the web pages.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
This also occurs for stylesheets added by the manifest.

You can get around it by creating a style element and adding it to the page manually, though this is obviously not ideal.
Duplicate of this bug: 1463666
Is there any update on this bug? It was confirmed almost 2 months ago and hasn't had anything added since.
This is still affecting me and anyone else adding stylesheets with no correspondence in months.
Flags: needinfo?(svoisen)
jwatt: Any thoughts here? Perhaps something to look into as part of recent printing investigation?
Flags: needinfo?(svoisen) → needinfo?(jwatt)
The WebExtensions implementation eventually calls into platform code to insert the sheet here:

https://searchfox.org/mozilla-central/rev/2fe43133dbc774dda668d4597174d73e3969181a/toolkit/components/extensions/ExtensionContent.jsm#446-455

That code uses WindowUtils::AddSheet, which calls nsIDocument::AddAdditionalStyleSheet, which adds the sheet to mAdditionalSheets[AGENT_SHEET]:

https://dxr.mozilla.org/mozilla-central/rev/c2e3be6a1dd352b969a45f0b85e87674e24ad284/dom/base/nsDocument.cpp#4413

Specifically, that is *mAdditionalSheets*.

When we actually clone the document for printing, we call nsIDocument::CreateStaticClone. That method does have code to clone the document's style sheets:

https://dxr.mozilla.org/mozilla-central/rev/c2e3be6a1dd352b969a45f0b85e87674e24ad284/dom/base/nsDocument.cpp#9551-9565

However, that code is only cloning from mStyleSheets, not from mAdditionalSheets. Therefore the sheets inserted by WebExtensions are not cloned.
Quite shockingly ( :P ), there is no documentation for the the 'additionalSheetType' enum, any of the methods for adding or removing "additional" sheets, or the mAdditionalSheets member, which makes it unclear how it differs from mStyleSheets...or whether these sheets should be cloned too. That lack of any documentation also applies to mStyleSheets.

Emilio, any idea whether I can go ahead and clone mAdditionalSheets across to the static clone, or if there are special considerations?
Flags: needinfo?(jwatt) → needinfo?(emilio)
I don't think there's anything special about mAdditionalSheets you should have into consideration, no...

Only gotcha that there could be is that depending on what the order is, if the cascade level is author, it could end up conflicting with the pages' style / end up in another order than the document, given we go through AppendStyleSheet unconditionally when you call AddAdditionalStyleSheet. But that doesn't sound like a huge issue to me.

Relatedly, the printing code is doing a deep clone of the stylesheets, when it only really needs a shallow clone (and mark the inner not to be unique)... Anyway that's something that we could improve memory usage wise, though I guess printing documents aren't long-lived generally so it may not matter much.
Flags: needinfo?(emilio)
Thanks, Emilio. I'll knock up a patch later today, and spin off a couple of bugs for adding documentation and improving the memory use.
Assignee: nobody → jwatt
Duplicate of this bug: 1417569
Comment on attachment 9006415 [details]
Bug 1448507 - Clone the additional style sheets when static cloning a document. r?xidorn!

Xidorn Quan [:xidorn] UTC+10 has approved the revision.
Attachment #9006415 - Flags: review+
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2168cbf8a779
Clone the additional style sheets when static cloning a document. r=xidorn
https://hg.mozilla.org/mozilla-central/rev/2168cbf8a779
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.