1700379 - Hit testing on fission iframes in print preview is messed up.

Status: RESOLVED FIXED

(Core :: Printing: Output, task)

Description

[Emilio Cobos Álvarez (:emilio)]

Because of the custom transforms stuff.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1700379 - Remove unused nsDisplayListBuilder::IsInPageSequence. r=miko

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1700379 - Move code dealing with transform getters to the frame that we actually wrap its contents around. r=miko

This shouldn't change behavior, but simplifies the setup quite a bit, I
think.

Depends on D109509

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1700379 - Make transform getters more like the other transforms. r=miko

This causes a couple issues because we're transforming the scrolled content of
the root scroll frame, and that's unexpected. But I have a follow-up patch that
should fix it.

Remove a bit of the aStyleDisplay gunk that shouldn't be needed because of two
reasons:

• Stylo is faster / has the style display one pointer-chase away.
• We check the MAY_BE_TRANSFORMED bit first now, and we deal with
  SVG-transformed frames, so for non-transformed frames IsTransformed should
  just be a bit-check now.

Depends on D109510

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1700379 - Remove -moz-scrolled-page-sequence. r=dholbert

This is needed for the previous patch, because otherwise we create a
reference frame with the root's scrolled content (the
::-moz-scrolled-page-sequence), and that breaks some display list
invariants.

Always create a canvas frame instead, (doesn't matter when printing),
and create a single ::-moz-page-sequence frame.

We need to fix PopulateReflowOutput because after the previous patch we
transform the whole frame, not only the contents.

Depends on D109511

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1acb0d8f6636
Remove unused nsDisplayListBuilder::IsInPageSequence. r=miko

Comment 6

[Phabricator Automation]

Comment on attachment 9211044 [details]
Bug 1700379 - Remove -moz-scrolled-page-sequence. r=dholbert

Revision D109512 was moved to bug 1700472. Setting attachment 9211044 [details] to obsolete.

Comment 7

[Phabricator Automation]

Comment on attachment 9211043 [details]
Bug 1700379 - Make transform getters more like the other transforms. r=miko

Revision D109511 was moved to bug 1700478. Setting attachment 9211043 [details] to obsolete.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1700379 - Make transform getters more like the other transforms. r=miko

This causes a couple issues because the "build overflow for extra pages"
stuff assumes that all pages are in the same reference frame and that's
no longer true...

Also, nsPageSequenceFrame::PopulateReflowOutput needs fixing because we start
transforming the whole frame, not just the contents.

Depends on D109510

Comment 9

[Emilio Cobos Álvarez (:emilio)]

So D109510 is green, but D109511 breaks the "Build the display list for previous page's overflow". The reason for this is that since the page frame starts being transformed, then it forms a reference frame, and that causes havoc because now coordinate spaces don't match. It seems we don't deal with differently-sized pages in any way, so maybe it's not hard to fix and it allows us to get rid of the "additional offset" thing... Miko, do you know?

We could fix this bug without it, by making nsIFrame::GetTransformMatrix deal with these frames specially. That's certainly an easier patch but unifying the transform behavior seemed nice.

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1acb0d8f6636

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0229f8dc291a
Move code dealing with transform getters to the frame that we actually wrap its contents around. r=miko

Comment 12

[Alexandru Michis [:malexandru]]

Backed out changeset 0229f8dc291a (Bug 1700379) for causing failures in test_printpreview.xhtml
Backout link: https://hg.mozilla.org/integration/autoland/rev/e10eee1d8fefd491062a9aab031cc8d50ddeef7c
Push with failures, failure log.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/029a5e8fa8f3
Move code dealing with transform getters to the frame that we actually wrap its contents around. r=miko

Comment 14

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/029a5e8fa8f3

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1700379 - Account for custom transforms in nsIFrame::GetTransformMatrix. r=miko

I don't have time to green up the previous patch (and it's not clear
it's such a big improvement anyways), so I'd rather do this for now.

Comment 17

[Mathew Hodson]

Since this has regression bugs marked against it and code landed n Firefox 90, I'm going to mark this as fixed so it's easier to tell which versions were affected by this and the other bugs.