1930618 - @scope on an inline node doesn't work when printing

Status: ASSIGNED

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

Description

[u723190]

Attachment: firefox_crash_scope.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:132.0) Gecko/20100101 Firefox/132.0

Steps to reproduce:

Open the attached HTML file and try to print it to PDF (at least under Fedora 39 with Gnome Shell 45).

I did submit the crash report, its ID is: 75c7440f-a031-a638-d493-454e8ca05a07

Actual results:

The tab crashed.

Expected results:

The page should have been printed to a PDF.

Comment 1

[u723190]

Addition: in "about:config", "layout.css.at-scope.enabled" has to be se to true.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Printing: Output' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Mayank Bansal]

confirmed.
I got a crash from the STR in comment #0. Crash: https://crash-stats.mozilla.org/report/index/cf91346b-7557-4426-ae2a-aa90a0241112

Comment 4

[Mayank Bansal]

Bisection:
Bug 1886441: Part 7 - Fast-reject with ancestor hashes for @scope. r=firefox-style-system-reviewers,emilio

Differential Revision: https://phabricator.services.mozilla.com/D208028

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1886441

Comment 6

[David Shin[:dshin]]

Hmmmmmmm...

So:

• When we're creating a document for printing, we generate a static clone of the document.
• That clone function then clones the DOM structure.
• When the <style> node is cloned, we skip attaching the sheet to it - makes sense, since that gets cloned later as the comment indicates.
• After the Document node is cloned, stylesheets are cloned here.

Note that there's no SetOwningNode() call - by this point, we lost information as to what this sheet's owning node is.
... Perhaps we need to handle sheet duplication in its Clone implementation, if it's part of the static document creation (If that's possible)?

Above leads to a situation where we know we have an implicit scope, but we can't find the stylesheet's owning node, which causes the crash.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Hmm, yeah we should fix up the stylesheet owner node.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

I guess this doesn't crash anymore after bug 1931054. S3 sounds fair but we should fix @scope in print.

Comment 9

[David Shin[:dshin]]

Attachment: Bug 1930618: `LinkStyle` subclasses clone its stylesheet while cloning for static document. r=#dom-core,#layout

... This preserves the link between the node and its stylesheet, enabling
implicit scope to continue working for the static document (e.g. For printing).

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1886441

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b5c3018b0d1
`LinkStyle` subclasses clone its stylesheet while cloning for static document. r=layout-reviewers,emilio

Comment 12

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/49379 for changes under testing/web-platform/tests

Comment 13

[agoloman]

Backed out for causing bc failures @/browser_preview_navigation.js.

• Backout link
• Push with failures
• Failure Log

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 15

[David Shin[:dshin]]

Prior to this patch, stylesheets cloned into a static document without the reference between them and relevant <style> and <link> elements preserved, which is the root cause of this bug.
... However, fixing this broke Print Selection, because it deletes all the nodes not selected as part of the printing process, which includes <style> and <link> elements. Now that those are linked to Stylesheet objects properly, they also get deleted, effectively leaving the selected element unstyled.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Grr... Yeah, that's quite annoying. It seems we'd have to keep <style> and <link> elements... At least the ones in the <head>? We should arguably never be removing the <head>, that might be the simplest fix in practice.

I guess that could cause some unintended side-effects. But the alternative would be to keep all stylesheets, which is also a bit weird because we have to keep their containers as well, and those might be arbitrary.

Maybe we can change the print selection code to not remove stuff but just hide it via CSS? (As in, set a style="display: none !important" on the relevant nodes or so. That might cause less unintended consequences (but also a bit more extra work compared to just deleting the nodes, because hidden <img> and co might load. Still probably fine).

Comment 17

[David Shin[:dshin]]

Grr... Yeah, that's quite annoying. It seems we'd have to keep <style> and <link> elements... At least the ones in the <head>? We should arguably never be removing the <head>, that might be the simplest fix in practice.

Would be incorrect for implicit scope, unfortunately (Since you'd want that to be down off of the DOM tree, out of <head> to really mean something). We could keep <style> and <link> elements down the descendant tree to the selection, which is slightly less arbitrary than keeping them all, but yeah, it's not great.

Maybe we can change the print selection code to not remove stuff but just hide it via CSS?

This may be the only way to have print selection style more correctly, honestly. Would fix bug 1933711, but then bug 1933715 looks fun.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Yeah, I think it's probably the least bad trade-off, given it fixes this and should be an improvement. In general, same styling between print and print selection is hard to guarantee (first-line is a good example, but there are others like container queries or what not). Still I agree it's better to try to keep the DOM as-is to minimize unexpected behavior.