Closed Bug 1552789 Opened 5 months ago Closed 5 months ago

Bug 1536778 renders absolutely positioned elements inside inline elements invisible

Categories

(Core :: Web Painting, defect, P1)

68 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 --- unaffected
firefox68 + fixed
firefox69 --- verified

People

(Reporter: denschub, Assigned: miko)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files)

Attached file Test case

Please see the attached test case. You should see a text You can see me., as well as a But you can't see me! text rendered somewhere below that. After bug 1536778 landed, you can't see the second element at all.

I discovered this while investigating a report where a dropdown menu in Sentry did not show up. Given the simplicity of the test case, this could be really bad.

Miko, could you have a look please?

Flags: needinfo?(mikokm)

(In reply to Dennis Schubert [:denschub] from comment #1)

Miko, could you have a look please?

Thank you for the report and testcase. I'll have a look.

Assignee: nobody → mikokm
Status: NEW → ASSIGNED
Flags: needinfo?(mikokm)
Priority: -- → P1

This was quite unexpected. It seems that in the testcase, building a display list for block frame lines changes the NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO state bit for the block frame, which affects whether we descend into successive lines or not.

This patch restores the original behavior of (re)checking the flag for every line.

Can you please explain how this happens?

My expectation is that we mark all placeholders with NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO up to their containing block, when we first encounter that containing block (before processing lines).

So when processing lines, I'd expect us to have already added flags for placeholders, except for ones within a nested containing block, but those shouldn't add flags outside of the containing block.

Which bit of that differs here?

Adding a frame tree dump might be useful.

Flags: needinfo?(mikokm)
Attached file frametree.txt

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)

Can you please explain how this happens?

My expectation is that we mark all placeholders with NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO up to their containing block, when we first encounter that containing block (before processing lines).

So when processing lines, I'd expect us to have already added flags for placeholders, except for ones within a nested containing block, but those shouldn't add flags outside of the containing block.

Which bit of that differs here?

Adding a frame tree dump might be useful.

Attached a frame tree dump.

I did not investigate this in more detail yet, I just wanted to have a patch ready for uplifting to 68. If the testcase is any indication, this can cause bad regressions.

I am unsure if this is related, but the testcase also triggers an assertion in layout
[Child 37916, Main Thread] WARNING: Out-of-flow frame got reflowed before its placeholder: file /layout/generic/nsPlaceholderFrame.cpp, line 135.
Emilio says that this assertion is related to bug 255139.

Flags: needinfo?(mikokm)

The invalid assumption in this case is that the frame that owns the OOF frames is also an ancestor of the placeholder. That is not true in this case (an IB split).

We should always reach the frame with OOF children during DL building (if visible), since the OOF frames contribute overflow area to that frame. That then marks our ancestor with the force descend into flag, which makes us descend into later lines that contain the placeholder.

So as long as the placeholder frames are always in a latter line than the OOFs (which I believe is the case, OOFs are always on the first line), then things should work correctly.

I still want us to fix bug 1460484 to simplify this, but it isn't necessary here.

Pushed by mikokm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/979750dd65da
Check NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO flag for every line r=mattwoodrow
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Please request beta uplift when you feel comfortable doing so. Thanks!

Has Regression Range: --- → yes
Has STR: --- → yes
Flags: qe-verify+
Flags: in-testsuite+

Comment on attachment 9066181 [details]
Bug 1552789 - Check NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO flag for every line r=mattwoodrow

Beta/Release Uplift Approval Request

  • User impact if declined: Certain websites with absolutely positioned elements inside inline elements might have elements missing.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The problem is well understood. The patch is simple, and just restores the previous behavior.
  • String changes made/needed:
Attachment #9066181 - Flags: approval-mozilla-beta?

Verified, that the issue is not reproducible on Nightly 69(20190524095337), using the attached test case.

Comment on attachment 9066181 [details]
Bug 1552789 - Check NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO flag for every line r=mattwoodrow

web painting regression fix, approved for 68.0b5

Attachment #9066181 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.