Summary: | An element having -webkit-mix-blend mode should only blend with the contents of the parent stacking context | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mihai Tica <mitica> | ||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, dino, esprehn+autocc, glenn, kondapallykalyan, mihnea, simon.fraser, WebkitBugTracker | ||||||||
Priority: | P2 | Keywords: | AdobeTracked | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 95614 | ||||||||||
Attachments: |
|
Description
Mihai Tica
2014-02-21 08:32:30 PST
Created attachment 224877 [details]
Patch V1
Comment on attachment 224877 [details] Patch V1 View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review > Source/WebCore/ChangeLog:9 > + This change isolates blending, preventing it to blend with other underlying elements besides the parent stacking context. Typo "to" -> "from" > Source/WebCore/rendering/RenderLayer.cpp:814 > + if (!oldStyle && renderer().style().hasBlendMode()) > + m_updateParentStackingContextShouldIsolateBlendingDirty = true; > + > + if (oldStyle && oldStyle->hasBlendMode() != renderer().style().hasBlendMode()) > + m_updateParentStackingContextShouldIsolateBlendingDirty = true; Make this one test, even though it will be long. > Source/WebCore/rendering/RenderLayer.h:785 > #if ENABLE(CSS_COMPOSITING) > bool hasBlendMode() const { return renderer().hasBlendMode(); } > + bool isolatesBlending() const { return m_isolatesBlending; } > #else > bool hasBlendMode() const { return false; } > + bool isolatesBlending() const { return false; } > #endif I think it would be better to write these the other way around: bool isolatesBlending() const { #if ENABLE(CSS_COMPOSITING) return m_isolatesBlending; #else return false; #endif } Same for hasBlendMode Created attachment 225034 [details]
Patch for landing
Comment on attachment 225034 [details] Patch for landing Rejecting attachment 225034 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 225034, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/5120381560029184 Created attachment 225042 [details]
Patch for landing
Comment on attachment 225042 [details] Patch for landing Clearing flags on attachment: 225042 Committed r164579: <http://trac.webkit.org/changeset/164579> Comment on attachment 224877 [details] Patch V1 View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review > Source/WebCore/rendering/RenderLayer.cpp:836 > + ancestor->m_isolatesBlending = renderer().style().hasBlendMode(); Would have been nice to call setIsolatesBlending() here. What ensures that m_isolatesBlending is updated when layers change between being stacking context and not? I don't see how the code currently can correctly maintain the m_isolatesBlending flag. (In reply to comment #7) > (From update of attachment 224877 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review > > > Source/WebCore/rendering/RenderLayer.cpp:836 > > + ancestor->m_isolatesBlending = renderer().style().hasBlendMode(); > > Would have been nice to call setIsolatesBlending() here. > > What ensures that m_isolatesBlending is updated when layers change between being stacking context and not? I don't see how the code currently can correctly maintain the m_isolatesBlending flag. You are right, I've mistakenly omitted this case, it should be corrected in a follow up patch. (In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 224877 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=224877&action=review > > > > > Source/WebCore/rendering/RenderLayer.cpp:836 > > > + ancestor->m_isolatesBlending = renderer().style().hasBlendMode(); > > > > Would have been nice to call setIsolatesBlending() here. > > > > What ensures that m_isolatesBlending is updated when layers change between being stacking context and not? I don't see how the code currently can correctly maintain the m_isolatesBlending flag. > > You are right, I've mistakenly omitted this case, it should be corrected in a follow up patch. Filled follow-up bug: https://bugs.webkit.org/show_bug.cgi?id=129307 Close this one as the follow up bug landed. |