Closed Bug 1700379 Opened 2 months ago Closed 2 days ago

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

Categories

(Core :: Printing: Output, task)

task

Tracking

()

RESOLVED DUPLICATE of bug 1710059
Fission Milestone M7a

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 2 obsolete files)

Because of the custom transforms stuff.

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

Depends on D109509

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

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

Keywords: leave-open
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1acb0d8f6636
Remove unused nsDisplayListBuilder::IsInPageSequence. r=miko
Summary: Hit testing on fission iframes is messed up. → Hit testing on fission iframes in print preview is messed up.
Depends on: 1700472

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.

Attachment #9211044 - Attachment is obsolete: true
Depends on: 1700478

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.

Attachment #9211043 - Attachment is obsolete: true

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

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.

No longer depends on: 1700478
Flags: needinfo?(mikokm)
Depends on: 1700478
Fission Milestone: --- → M7a
Flags: needinfo?(mikokm)
Pushed by ealvarez@mozilla.com:
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

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.

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
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

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.

Status: NEW → RESOLVED
Closed: 2 days ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1710059
You need to log in before you can comment on or make changes to this bug.