Bug 129480

Summary:

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

Product:

WebKit

Reporter:

Mihai Tica <mitica>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

allan.jensen, bfulgham, buildbot, clopez, commit-queue, dbates, eocanha, esprehn+autocc, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, mihnea, rniwa, simon.fraser, webkit-bug-importer, WebkitBugTracker

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=287407

Attachments:

Description	Flags
Sample reproducing the issue	none
Patch	mcatanzaro: review-, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion	none

Mihai Tica

Reported 2014-02-28 05:35:42 PST

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.

Attachments
Sample reproducing the issue (665 bytes, text/html) 2014-02-28 05:36 PST, Mihai Tica	no flags	Details
Patch (10.75 KB, patch) 2014-03-28 06:36 PDT, Enrique Ocaña	mcatanzaro: review- buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (456.54 KB, application/zip) 2014-03-28 08:33 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (487.48 KB, application/zip) 2014-03-28 10:07 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (548.69 KB, application/zip) 2014-03-28 10:53 PDT, Build Bot	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Mihai Tica

Comment 1 2014-02-28 05:36:19 PST

Created attachment 225454 [details] Sample reproducing the issue

Enrique Ocaña

Comment 2 2014-03-28 06:36:43 PDT

Created attachment 228046 [details] Patch

Build Bot

Comment 3 2014-03-28 08:33:13 PDT

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

Comment 4 2014-03-28 08:33:19 PDT

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 5 2014-03-28 10:06:56 PDT

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

Comment 6 2014-03-28 10:07:05 PDT

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 7 2014-03-28 10:20:53 PDT

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 8 2014-03-28 10:53:12 PDT

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

Comment 9 2014-03-28 10:53:19 PDT

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

Comment 10 2014-03-28 14:07:12 PDT

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

Comment 11 2014-03-28 14:23:45 PDT

(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 12 2016-01-02 09:26:16 PST

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

Comment 13 2022-07-13 17:09:41 PDT

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

Radar WebKit Bug Importer

Comment 14 2022-07-13 17:09:58 PDT

<rdar://problem/96982301>

Note You need to log in before you can comment on or make changes to this bug.