1653334 - "Print selection" is broken if the selected text is in a subdocument (iframe)

Status: VERIFIED FIXED

(Core :: Printing: Setup, defect, P1)

Description

[Jonathan Watt [:jwatt]]

We only call CachePrintSelectionRanges in nsPrintObject::InitAsRootObject. So if the selection is in a subdocument, then the "Print selection" functionality will be broken.

Comment 1

[Bob Owen (:bobowen) PTO until 8th Oct]

Looks like this is just a matter of adding the function call to do the caching into InitAsNestedObject.

Comment 2

[Jonathan Watt [:jwatt]]

I also had in mind to propagate out from the InitAs*Object functions the information on whether there is a selection. That would allow later code to avoid querying the documents. Ideally nsPrintData would know exactly which of its nsPrintObjects has the selection it should use. I believe that should be the one from the document that was cloned from the focused window, or else the selection from the first document found in a depth first search.

Comment 3

[Bob Owen (:bobowen) PTO until 8th Oct]

Attachment: Bug 1653334 part 1: Remove unused isRangeSelection and isIFrameSelected. r=jwatt!

These unused members are removed from nsIWebBrowserPrint and PPrintingTypes
along with their supporting functions.

Comment 4

[Bob Owen (:bobowen) PTO until 8th Oct]

Attachment: Bug 1653334 part 2: Cache the selection ranges on subdocuments as we build the nsPrintObject tree. r=jwatt!

This also refactors the selection printing code, so that as we build the tree we
record which nsPrintObject should be used if printing a Selection is chosen.

Depends on D85007

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75a5873f02a4
part 1: Remove unused isRangeSelection and isIFrameSelected. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/6cc6bbaa91f0
part 2: Cache the selection ranges on subdocuments as we build the nsPrintObject tree. r=jwatt

Comment 6

[Atila Butkovits]

Backed outfor reftest failures at test-print-selection.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/e3d4f0ab31787a5d772c29ed7cb337ab9206a745

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=ataN657QQme7QcG4BHbJFQ.0&revision=6cc6bbaa91f09fa58f6746418305b9f9702e2de4&searchStr=os%2Cx%2C10.14%2Copt%2Creftests%2Ctest-macosx1014-64%2Fopt-reftest-e10s%2Cr4

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311871893&repo=autoland&lineNumber=8623

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93dd13edad8a
part 1: Remove unused isRangeSelection and isIFrameSelected. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/2adc23aed2a6
part 2: Cache the selection ranges on subdocuments as we build the nsPrintObject tree. r=jwatt

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/93dd13edad8a
https://hg.mozilla.org/mozilla-central/rev/2adc23aed2a6

Comment 9

[Julien Cristau [:jcristau]]

Is this worth uplifting to beta and/or esr78?

Comment 10

[Bob Owen (:bobowen) PTO until 8th Oct]

(In reply to Julien Cristau [:jcristau] from comment #9)

Is this worth uplifting to beta and/or esr78?

This turned into quite a big change, so I'm a little nervous about uplifting to Beta now that Beta is quite short.
The crash that this fixes in bug 1653981 is fairly rare.
jwatt - what do you think?

I think once it looks good on Beta (but maybe Beta 81), we should probably uplift to ESR78.

Comment 11

[Jonathan Watt [:jwatt]]

I agree with all the above.

Comment 12

[Julien Cristau [:jcristau]]

Thank you!

Comment 13

[Anca Soncutean, Desktop QA]

Reproduced with Fx 80.0a1 (2020-07-16) on Windows 10.
Verified fixed with Fx 81.0a1 (2020-08-18) on Windows 10, macOS 10.15 and Ubuntu 18.04.