1. Set svg.style.backgroundColor = "blue" 2. Notice the svg element does not change 3. Resize the window and notice the background updates Testcase below: <!DOCTYPE HTML> <html> <head> <script> function test() { var svg = document.getElementById('svg'); setTimeout(function() { svg.style.backgroundColor = "green"; }, 100); } </script> </head> <body onload="test()"> This test passes if there is a green rectangle below:<br/> <svg id="svg" width="100px" height="100px"> </svg> </body> </html> This has been fixed in Blink. We just need to merge the patch from crbug.com/354305.
<rdar://problem/22991057>
Created attachment 262758 [details] Patch
Comment on attachment 262758 [details] Patch Attachment 262758 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/260872 New failing tests: svg/overflow/overflow-on-outermost-svg-element-in-xhtml-visible.xhtml svg/animations/animate-viewport-overflow.html fast/repaint/moving-shadow-on-container.html svg/animations/animate-viewport-overflow-2.html
Created attachment 262760 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 262758 [details] Patch Attachment 262758 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/260892 New failing tests: svg/overflow/overflow-on-outermost-svg-element-in-xhtml-visible.xhtml svg/animations/animate-viewport-overflow.html fast/repaint/moving-shadow-on-container.html svg/animations/animate-viewport-overflow-2.html
Created attachment 262761 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 262765 [details] Patch
Comment on attachment 262765 [details] Patch Attachment 262765 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/261138 New failing tests: svg/overflow/overflow-on-outermost-svg-element-in-xhtml-visible.xhtml svg/animations/animate-viewport-overflow.html svg/animations/animate-viewport-overflow-2.html
Created attachment 262767 [details] Archive of layout-test-results from ews105 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Comment on attachment 262765 [details] Patch Attachment 262765 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/261168 New failing tests: svg/overflow/overflow-on-outermost-svg-element-in-xhtml-visible.xhtml svg/animations/animate-viewport-overflow.html svg/animations/animate-viewport-overflow-2.html
Created attachment 262768 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
The newly failing tests are all using an `overflow: visible` style, so this is most likely where the issue is lying. Can a reviewer spot what in the changes made to RenderSVGRoot.cpp would cause repaints to areas outside of the overflowing area not to be performed?
(In reply to comment #12) > The newly failing tests are all using an `overflow: visible` style, so this > is most likely where the issue is lying. Can a reviewer spot what in the > changes made to RenderSVGRoot.cpp would cause repaints to areas outside of > the overflowing area not to be performed? Never mind that, the issue was pretty clear, overflow handling was already performed in RenderSVGRoot::layout() if shouldApplyViewportClip() was false and we were now resetting the overflow in all cases. Removing the new code for that purpose should fix things right up.
Created attachment 262772 [details] Patch
Comment on attachment 262772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262772&action=review > Source/WebCore/rendering/svg/RenderSVGRoot.cpp:309 > if (diff == StyleDifferenceLayout) > setNeedsBoundariesUpdate(); > + > + if (diff == StyleDifferenceRepaint) { > + // Box decorations may have appeared/disappeared - recompute status. > + m_hasBoxDecorations = hasBoxDecorationStyle(); > + } > RenderReplaced::styleDidChange(diff, oldStyle); > SVGResourcesCache::clientStyleChanged(*this, diff, style()); If adding a blank line before this new if block, then I suggest also adding a blank line after it. Both or neither. > Source/WebCore/rendering/svg/RenderSVGRoot.cpp:355 > + // Return early for any cases where we don't actually paint. All the comments in this function are too “on the nose”, stating what the code below does and we don’t find those comments as valuable as comments saying things that the code does *not* say. They can be improved by focusing more on *why* the code is needed, and minimize stating what the code does. > Source/WebCore/rendering/svg/RenderSVGRoot.cpp:361 > + FloatRect contentRepaintRect = repaintRectInLocalCoordinates(); > + contentRepaintRect = m_localToBorderBoxTransform.mapRect(contentRepaintRect); Might read better as a single line. > Source/WebCore/rendering/svg/RenderSVGRoot.cpp:372 > + // The selectionRect can project outside of the overflowRect, so take their union > + // for repainting to avoid selection painting glitches. > + LayoutRect decoratedRepaintRect = unionRect(localSelectionRect(false), visualOverflowRect()); > + repaintRect.unite(decoratedRepaintRect); Why not two calls to repaintRect.unite instead? > Source/WebCore/rendering/svg/RenderSVGRoot.cpp:377 > + LayoutRect rect = enclosingIntRect(repaintRect); > + return RenderReplaced::computeRectForRepaint(rect, repaintContainer); Would read better without a local variable and on a single line.
Created attachment 262785 [details] Patch for landing
Addressing the comments in the landing commit, thanks for the review Darin.
Comment on attachment 262785 [details] Patch for landing Clearing flags on attachment: 262785 Committed r190816: <http://trac.webkit.org/changeset/190816>
All reviewed patches have been landed. Closing bug.
The equivalent Windows test expectation was not updated. Committed r191116: <http://trac.webkit.org/changeset/191116>
Thanks Brent!