1552789 - Bug 1536778 renders absolutely positioned elements inside inline elements invisible

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P1)

Description

[Dennis Schubert [:denschub]]

Attachment: 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.

Comment 1

[Dennis Schubert [:denschub]]

Miko, could you have a look please?

Comment 2

[Miko Mynttinen]

(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.

Comment 3

[Miko Mynttinen]

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.

Comment 4

[Miko Mynttinen]

Attachment: Bug 1552789 - Check NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO flag for every line r=mattwoodrow

Comment 5

[Matt Woodrow (:mattwoodrow)]

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.

Comment 6

[Miko Mynttinen]

Attachment: frametree.txt

Comment 7

[Miko Mynttinen]

(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.

Comment 8

[Miko Mynttinen]

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.

Comment 9

[Matt Woodrow (:mattwoodrow)]

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.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/979750dd65da
Check NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO flag for every line r=mattwoodrow

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/979750dd65da

Comment 12

[Julien Cristau [:jcristau]]

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

Comment 13

[Miko Mynttinen]

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:

Comment 14

[Gyula Palko[:gpalko]]

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

Comment 15

[Julien Cristau [:jcristau]]

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

Comment 16

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/413832ca33c2