UpdateTransformLayer and UpdateOpacityLayer change hints incorrectly merged by style change processing

RESOLVED FIXED in mozilla13

Status

()

Core
Layout
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
mozilla13
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

ReResolveStyleContext assumes, in general, that when we generate a style change list entry for a frame with a given hint, there is no need to generate style change list entries for descendant frames with the same hint.

This is wrong for UpdateTransformLayer and UpdateOpacityLayer and I think it explains a bug Nick Cameron showed me the other day.
Created attachment 594668 [details]
testcase

Load the testcase. After you see that the lime DIV has faded a bit, click away to another window to force the page to be fully repainted. You'll see the yellow DIV suddenly snap to its correct rendering.
We have two options here.

Option 1 is to add a clause to the code at the beginning of ReResolveStyleContext that handles this situation for the reflow flags to remove the layer flags from aMinChange.

Option 2 is to change processing of the layer hints to walk all the kids (like the repaint hint does, say).

I'm guessing #1 is better?

Comment 3

6 years ago
Created attachment 594778 [details]
another test case

Hover over the yellow div to see the bug at the end of the transition
Created attachment 594885 [details] [diff] [review]
fix
Assignee: nobody → roc
Attachment #594885 - Flags: review?(matspal)
Created attachment 594886 [details] [diff] [review]
correct patch

Sorry about that.
Attachment #594885 - Attachment is obsolete: true
Attachment #594885 - Flags: review?(matspal)
Attachment #594886 - Flags: review?(matspal)
Comment on attachment 594886 [details] [diff] [review]
correct patch

Add a comment about how those don't get automatically applied to descendants, and r=me
Attachment #594886 - Flags: review?(matspal) → review+
Thanks. I'll wait for Mats to land his stuff before landing this.
FYI, I landed my stuff directly on mozilla-central so make sure it is merged first
if you want to land on mozilla-inbound.

Also, is there any situation where a child has opacity:inherit and the rule nodes
are shared so that nsStyleDisplay::CalcDifference isn't called, like we saw in
bug 723669.... Actually, never mind, I see that nsStyleDisplay has:
  static bool ForceCompare() { return true; }
so that covers both UpdateTransformLayer and UpdateOpacityLayer since they are only
used by nsStyleDisplay::CalcDifference.  Can we document this dependency somehow?
It is documented in nsStyleStruct.h:
   Structs that can return a hint that doesn't guarantee that the
// change will be applied to all descendants must return true from
// ForceCompare(), so that we will make sure to compare those structs in
// nsStyleContext::CalcStyleDifference.

I'll document it in nsChangeHint.h as well.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd5cba44fa8e
https://hg.mozilla.org/mozilla-central/rev/bd5cba44fa8e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.