Closed Bug 1805908 Opened 2 years ago Closed 2 years ago

Printing selected text from the section Titles in Firefox View will keep Print Preview in infinite loading

Categories

(Core :: Print Preview, defect, P3)

Desktop
All
defect

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox-esr102 --- disabled
firefox108 --- wontfix
firefox109 --- wontfix
firefox110 --- wontfix

People

(Reporter: rdoghi, Unassigned)

References

(Blocks 2 open bugs, Regression)

Details

(Keywords: regression, Whiteboard: [fidefe-fxview-polish])

Attachments

(2 files)

Attached video 2022-12-15_13h13_40.mp4 (deleted) —

Found in

  • Beta 109.0b2 (2022-12-15)

Affected versions

  • Firefox Nightly 110.0a1 (2022-12-15)
  • Beta 109.0b2
  • Release 108

Affected platforms

  • All

Steps to reproduce

  1. Reach the Firefox View page.
  2. Select the Text from the page that also contains part of the Recently Closed or Tab pickup titles.
  3. Right Click the selected text and select Print Selection from the Context menu.

Expected result

  • Section titles should be displayed in Print Preview.

Actual result

  • If any part of the Title text is selected, Print Preview remains in infinite loading.

Regression range
8:06.11 INFO: No more integration revisions, bisection finished.
8:06.12 INFO: Last good revision: 4e5ff4cd430a60a5b44b86eb25f8de4a17e45b4b
8:06.12 INFO: First bad revision: fff0a3464faef799db0b1c94e3053ad0e4460c9d
8:06.12 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4e5ff4cd430a60a5b44b86eb25f8de4a17e45b4b&tochange=fff0a3464faef799db0b1c94e3053ad0e4460c9d

Hi @Gijs can you take a look at this issue it seems 1780239 is whats causing it.

Flags: needinfo?(gijskruitbosch+bugs)
Regressed by: 1780239
Whiteboard: [fidfe-fxview-polish]
Whiteboard: [fidfe-fxview-polish] → [fidefe-fxview-polish]
Priority: -- → P3

Adding Bernard for context as he wrote the regressing patch based on the approach I suggested in the bug.

The following errors appear in the browser console:

18:06:53.651
UnknownError: Print preview failed 2 printUtils.js:724:59
_printPreview chrome://global/content/printUtils.js:724
printPreview chrome://global/content/printUtils.js:667
_updatePrintPreview chrome://global/content/print.js:831
init chrome://global/content/print.js:323
AsyncFunctionNext self-hosted:807
18:06:53.663
TypeError: browser is nullPrintingParent.jsm:14:7
receiveMessage resource://gre/actors/PrintingParent.jsm:14

I expect this isn't Firefox View's own fault, tbh, and the regressing bug just happened to trip over a pre-existing issue with print preview. Maybe Mark can help based on his print preview experience?

Component: Firefox View → Print Preview
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mstriemer)
Product: Firefox → Core

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Priority: P3 → --
Priority: -- → P3

Drive-by observation, while was taking a look at these suspicious assertions that accompany the preview:

[Parent 52770, Main Thread] WARNING: '!sr', file /home/dshin/mozilla-unified/dom/html/HTMLDetailsElement.cpp:68
[Parent 52770, Main Thread] WARNING: '!sr', file /home/dshin/mozilla-unified/dom/html/HTMLDetailsElement.cpp:68

I noticed that breaking on HTMLDetailsElement::SetupShadowTree and just continuing causes the preview to render correctly (?!)

Similar to bug 1806913, it seems like Firefox View might have a resize event-handler that messes with a DOM a bit and causes selections to be lost (and printing does count as a resize and probably triggers this event handler).

From some quick testing:

  • If select some of the "Reopen pages" text and then resize my browser to be wider & skinnier, then my selection gets lost at some point (not always right away).

  • If I have Firefox View in its skinny single-column form (with Colorways in a single column with the other content) and perform the STR here, Print Selection seems to work just fine (presumably because there's no resize across a meaningful threshold to trigger whatever the DOM-disturbing behavior is; the printed page is roughly the same size as the "skinny window" in terms of CSS pixels).

(In reply to Daniel Holbert [:dholbert] from comment #5)

Similar to bug 1806913, it seems like Firefox View might have a resize event-handler that messes with a DOM a bit and causes selections to be lost (and printing does count as a resize and probably triggers this event handler).

From some quick testing:

  • If select some of the "Reopen pages" text and then resize my browser to be wider & skinnier, then my selection gets lost at some point (not always right away).

  • If I have Firefox View in its skinny single-column form (with Colorways in a single column with the other content) and perform the STR here, Print Selection seems to work just fine (presumably because there's no resize across a meaningful threshold to trigger whatever the DOM-disturbing behavior is; the printed page is roughly the same size as the "skinny window" in terms of CSS pixels).

Not a resize event listener but a matchMedia listener which seemed like it would avoid having to debounce the resize listener and such, and ensures that we stay in sync with the CSS (well, ideally).

Is it possible to check if the resize / media query listener invocation is caused by print preview, e.g. by checking a different media query? :-)

Flags: needinfo?(dholbert)
Attached file Example with media query listener (deleted) —

(Drive-by since I looked at bug 1806913 already)
Here's an example with media query listener.
Error message is not popped because the message path here is broken. Console shows JavaScript error: resource://gre/actors/PrintingParent.jsm, line 14: TypeError: can't access property "ownerGlobal", browser is null. I think it's due to being a mainthread based tab, but the "unbroken" behaviour isn't really better as shown in this test case as well as bug 1806913.

Flags: needinfo?(dholbert)

Should I assume based on comment 7 and bug 1806913 that we're going to try to fix the print code rather than the consumers like Firefox View and google calendar? :-)

I thought we disabled JS in the print document, so I wonder if that's not happening in time and/or if the resize/mql listeners being queued is somehow bypassing that. Or perhaps I'm just wrong and we don't disable JS in print docs? :-)

(In reply to :Gijs (he/him) from comment #8)

Is it possible to check if the resize / media query listener invocation is caused by print preview, e.g. by checking a different media query? :-)

You might be able to opt out of firing on print if you add not print to the beginning of your string...
e.g. this media-query applies to all viewports wider than 50px which are not print documents

data:text/html,<style>@media not print and (min-width: 50px) { div { text-decoration:underline}}</style><div>hi

I imagine the equivalent matchMedia query would have the same behavior (not firing on print).

Should I assume based on comment 7 and bug 1806913 that we're going to try to fix the print code rather than the consumers like Firefox View and google calendar? :-)

We probably need a general fix, yeah. It's possible that it won't be super-graceful for print-selection if the document actively messes with the part of the DOM that includes the selection (spitballing: maybe we expand the selection at that point, since we've potentially lost information).

Having said that: if Firefox View is able to either reduce its DOM-fiddling or skip its matchMedia callback on print (via e.g. adjusting the media query with not print or something to that effect), it might still be a nice way to reduce chaos and avoid less-than-graceful print-selection fallback (to the extent that we care about print-selection reliably doing precisely the right thing for Firefox View at least...)

I thought we disabled JS in the print document, so I wonder if that's not happening in time and/or if the resize/mql listeners being queued is somehow bypassing that. Or perhaps I'm just wrong and we don't disable JS in print docs? :-)

We do run matchMedia JS callbacks, as of recently -- see bug 774398. Some sites (e.g. CultureAmp) use matchMedia to do their CSS media-queries, basically; and they end up looking broken (more broken than other browsers) if we print their formatted-for-a-wide-viewport content. So: we fire matchMedia on print like other browsers do now.

I'm OK closing this one for now as it's not high priority at the moment for this particular feature.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: