Closed Bug 1514291 Opened 6 years ago Closed 3 years ago

Incorrect clipping behaviour of fixed element inside sticky element

Categories

(Core :: Web Painting, defect, P3)

Product:
Core
Component:
Web Painting
Platform:
Desktop
All
Type:
defect

Tracking

()

Status:
RESOLVED WONTFIX
Tracking Flags:
Tracking Status
firefox-esr60 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- wontfix
firefox68 --- wontfix

People

(Reporter: botond, Unassigned)

References

(
URL
)

Details

(Keywords: regression)

Attachments

(1 file)

Reftest
(deleted), patch
Details | Diff | Splinter Review 
STR:

  1. Load https://bug1214151.bmoattachments.org/attachment.cgi?id=8673036
  2. Scroll down until the black line reaches its topmost position
  3. Click on the page to force a repaint.
  4. Scroll back up

Expected results:

  The blue square is always rendered.

Actual results:

  During step 4, the blue square disappears.
  Clicking the page to force another repaint makes it re-appear.

This is a regression; it worked fine when I originally fixed bug 1214151.

Mozregression points to this range:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ce1a86d3b4db161c95d1147676bbed839d7a4732&tochange=9056f2ee492fa481aa86146aba236c074628e9fd

It doesn't seem to have builds for individual pushes in that range, but the range contains bug 1404181 (retained display lists), making it (I think) a natural suspect. However, setting layout.display-list.retain=false on a recent Nightly build does not make the problem go away (suggesting that perhaps it's not the retaining itself, but some of preparatory refactorings in that patch series that caused it?)
I am attempting to triage this issue and set the affected/unaffected version flags. This issue could be reproduced with the steps from comment 0 and it occurs on all main versions and platforms. Thank you.
status-firefox64: --- → affected
status-firefox65: --- → affected
status-firefox-esr60: --- → affected
OS: Unspecified → All
Hardware: Unspecified → Desktop

I think this bug might caused by the change 1 (in bug 1404181) to, and later (in bug 1426653) removal of, nsFrame::SchedulePaint() call, when mAPZFocusSequenceNumber changes. Adding this line back seems to fix the testcase locally (with some flickering due to constant repainting).

Unfortunately, I doubt that this is the right way to fix this. The root cause is probably a wrong display port, frame, or clip rect size, when the scrolled item is built.

Priority: -- → P2
Attached patch Reftest (deleted) — — Splinter Review

I turned the testcase into a reftest.

Some brief investigation shows that transforms are being set correctly in AsyncCompositionManager. In the reftest, we have a reftest-async-scroll-y=1000, so the sticky element (which is within its scrolling range) gets a transform of -1000, and the fixed element inside it gets a compensating transform of +1000 to keep it fixed.

The problem seems to be that in between those two transforms (at the level of the fixed container layer, which has the +1000 transform applied to it while inheriting the -1000 from its parent), the content is clipped to the viewport, which clips away the border which is "offscreen" at this level.

It seems to me that applying a viewport clip to a fixed layer that's inside something that can scroll (like the sticky layer in this case) is incorrect. That suggests the issue is with the clip / ASR computation code in Layout. I'd rather not touch that until after the cleanup planned in bug 1511419.

Depends on: 1511419

(Based on comment 2, bug 1404181 is a red herring. The change made there was just causing us to repaint more often, thereby avoiding a situation where we have a large enough async scroll offset for the content to be outside the viewport clip.)

No longer blocks: 1404181

Matt, this is a P2 without an assignee. Can you please find an assignee for this?

Flags: needinfo?(matt.woodrow)

(FWIW, I think this can be a P3 in the absence of a real-world website where this issue occurs.)

(In reply to Neha Kochar [:neha] from comment #7)

Matt, this is a P2 without an assignee. Can you please find an assignee for this?

I'll make this P3 as Botond suggested, but in general requiring P2s to be assigned isn't something we've been aiming for in Web Painting.

Flags: needinfo?(matt.woodrow)
Priority: P2 → P3

WONTFIX as this bug only occurs with non-WR.

Blocks: fixed-by-webrender
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
No longer blocks: fixed-by-webrender
Depends on: fixed-by-webrender
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: