1417947 - mac-its.com: Menu becomes blank when hovering over things that are not a link

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Tobias (:Tobbi) Markus]

Steps to reproduce:
1. Go to https://www.mac-its.com/
2. Hover over one of the menu items at the top.
3. Hover over the white menu that appears.

Results:

The menu items of the upcoming menu are disappearing, leaving only a blank surface.

Expected results:
The menu items should still be visible while hovering.

This works as expected on both Safari and Google Chrome.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

This seems stylo and animation issues.

Comment 2

[Brian Birtles (:birtles)]

Seems to be a recent regression

Comment 3

[Emilio Cobos Álvarez (:emilio)]

I suspect retained display lists instead of stylo, fwiw, but can take a look.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Yeah, works with retained-dl off.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Don't use MarkCurrentFrameModifiedDuringBuilding before we setup the current frame

This caller happens before the AutoBuildingDisplayList is initialized, so |this| isn't actually the current frame.

If we hit this branch on the root frame, then there's nothing on the stack to restore the value of nsDisplayListBuilder::mInInvalidSubtree.

So we have an nsDisplayBlendContainer for the root frame, which causes mInInvalidSubtree to get stuck at true for future paints, but we're also invalidating everything, so it's correct.

Then we remove the blend container, and try do a real partial build, but mInInvalidSubtree is still true, so we don't use the override dirty regions and build less than we should.

The new assertion would have caught this.

Comment 6

[Miko Mynttinen]

Comment on attachment 8929921 [details] [diff] [review]
Don't use MarkCurrentFrameModifiedDuringBuilding before we setup the current frame

Review of attachment 8929921 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. If it's not too much work, could we have a test case for this?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b45d85aef732
Mark the correct frame as modified when invalidating due to blend mode. r=miko

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b45d85aef732

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8929921 [details] [diff] [review]
Don't use MarkCurrentFrameModifiedDuringBuilding before we setup the current frame

Approval Request Comment
[Feature/Bug causing the regression]: bug 1352499. This is code that is preffed off, but we want to run a shield study enabling the pref.
[User impact if declined]: None, preffed off code.
[Is this code covered by automated tests?]: Yes, when the pref is enabled.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Code is preffed off.
[String changes made/needed]: None

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8929921 [details] [diff] [review]
Don't use MarkCurrentFrameModifiedDuringBuilding before we setup the current frame

Support retain display lists shield study which is pref-off now. Beta58+.

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c88f2766d765