Bug 129480 – Child elements aren't fully repainted when setting opacity from script

• Component: CSS
• Status: NEW

[Mihai Tica]

Consider the following document:

<div id="parent">
   <div id="child" class="overflowsParent createsStackingContext">
   </div>
</div>

Setting the parent's opacity from script doesn't trigger a full repaint of the child. Instead, only the fragments that intersect the parent are painted.

See attached test case. After the opacity is changed (about 1s), observe the rendering defect. Resizing the window to see the desired result.

This is probably caused by the repaintRect not including the children elements. When detecting such a change, we should hook into RenderLayer::computeRepaintRectsIncludingDescendants.
Note that this problem is only present when the child creates a stacking context.

This problem should be fixed for all properties that cause transparency: opacity, isolation, blend modes. In other words, we should repaint the descendants when detecting a change in the layer transparency.

[Mihai Tica]

Created attachment 225454 [details]
Sample reproducing the issue

[Enrique Ocaña]

Created attachment 228046 [details]
Patch

[Build Bot]

Comment on attachment 228046 [details]
Patch

Attachment 228046 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5131992635015168

New failing tests:
fast/repaint/blend-mode-isolate-stacking-context.html

[Build Bot]

Created attachment 228052 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 228046 [details]
Patch

Attachment 228046 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5405030148472832

New failing tests:
fast/repaint/blend-mode-isolate-stacking-context.html

[Build Bot]

Created attachment 228059 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Simon Fraser (smfr)]

Comment on attachment 228046 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228046&action=review

> Source/WebCore/css/StyleResolver.cpp:1143
> +        style.setEffectiveOpacity(style.opacity() * parentStyle.opacity());

Is "parentStyle" the correct one to be getting opacity from in all cases?

[Build Bot]

Comment on attachment 228046 [details]
Patch

Attachment 228046 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4701668587339776

New failing tests:
fast/repaint/blend-mode-isolate-stacking-context.html

[Build Bot]

Created attachment 228069 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Enrique Ocaña]

(In reply to comment #7)

> (From update of attachment 228046 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228046&action=review
> 
> > Source/WebCore/css/StyleResolver.cpp:1143
> > +        style.setEffectiveOpacity(style.opacity() * parentStyle.opacity());
> 
> Is "parentStyle" the correct one to be getting opacity from in all cases?

Thanks for pointing this out, Simon. It shouldn't be parentStyle.opacity(), but parentStyle.effectiveOpacity(), which is the field that "accumulates" opacity and represents the combined opacity() of all the ancestors.

[Simon Fraser (smfr)]

(In reply to comment #10)
> (In reply to comment #7)
> 
> > (From update of attachment 228046 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=228046&action=review
> > 
> > > Source/WebCore/css/StyleResolver.cpp:1143
> > > +        style.setEffectiveOpacity(style.opacity() * parentStyle.opacity());
> > 
> > Is "parentStyle" the correct one to be getting opacity from in all cases?
> 
> Thanks for pointing this out, Simon. It shouldn't be parentStyle.opacity(), but parentStyle.effectiveOpacity(), which is the field that "accumulates" opacity and represents the combined opacity() of all the ancestors.

Not just that, but I'm not convinced that the parent element is always the one you multiply style from (assuming that parentStyle is really the parent element's style). E.g. in some configurations of positioning and opacity.

[Michael Catanzaro]

Comment on attachment 228046 [details]
Patch

Seems from the comments that Simon has found a bug in this patch, so I'm removing this from request queue.

[Brent Fulgham]

Chrome and Firefox agree on the behavior here, but Safari seems to miss the final element.

[Radar WebKit Bug Importer]

<rdar://problem/96982301>