Bug 111927

Summary: Style updates are not propagated to <use> elements after creation
Product: WebKit Reporter: Stuart P. Bentley <stuart>
Component: SVGAssignee: Takashi Sakamoto <tasak>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: ahmad.saleem792, ajvincent, bfulgham, dglazkov, d-r, esprehn+autocc, fmalita, koivisto, macpherson, maggotfish, menard, ojan.autocc, pdr, sabouhallawa, schenney, simon.fraser, tanaka.toshiyuki, tasak, webkit-bug-importer, webkit.review.bot, zimmermann
Priority: P2 Keywords: InRadar, LayerBasedSVGEngine
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=255114
Attachments:
Description Flags
Testcase
none
Patch
none
Patch schenney: review+

Stuart P. Bentley
Reported 2013-03-09 12:53:16 PST
Created attachment 192349 [details] Testcase Changing the style of an element through the DOM does not update the style of any <use> elements referencing that element if the style is updated after creation. Repro: See test case. The change of the <circle>'s fill from green to navy that happens at initialization is copied by the <use> element, but the change on the timeout from navy to blue is not.
Attachments
Testcase (371 bytes, image/svg+xml)
2013-03-09 12:53 PST, Stuart P. Bentley 		no flags
Details
Patch (6.28 KB, patch)
2013-03-11 01:25 PDT, Takashi Sakamoto 		no flags
DetailsFormatted DiffDiff
Patch (6.34 KB, patch)
2013-03-12 01:55 PDT, Takashi Sakamoto 		schenney: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Stuart P. Bentley
Comment 1 2013-03-09 14:34:36 PST
Expected testcase behavior: Both circles turn blue (#00F). Actual testcase behavior: The original circle on the left turns blue (#00F), while the <use> element referencing it on the right remains navy (#008). <use> elements do currently update when the corresponding attributes are updated, so scripts setting style properties (ie. `element.style.fill = '#foo'`) can be rewritten to set attributes (ie. `element.setAttribute('fill','#foo')`) as a workaround. Original bug: https://code.google.com/p/chromium/issues/detail?id=166438
Takashi Sakamoto
Comment 2 2013-03-11 01:25:20 PDT
Created attachment 192422 [details] Patch
WebKit Review Bot
Comment 3 2013-03-11 02:29:33 PDT
Comment on attachment 192422 [details] Patch Attachment 192422 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17024155 New failing tests: html5lib/generated/run-tests16-data.html
Stephen Chenney
Comment 4 2013-03-11 07:33:32 PDT
Comment on attachment 192422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192422&action=review Basically good. Just fix the build for SVG disabled. The EWS chromium bot failure is a flake, not of any concern. > Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:31 > +#include "SVGElementInstance.h" This needs to be protected with #if ENABLE(SVG) > Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:403 > + SVGElementInstance::invalidateAllInstancesOfElement(toSVGElement(m_parentElement), false); This also needs #if ENABLE(SVG) protection.
Stephen Chenney
Comment 5 2013-03-11 07:35:03 PDT
Comment on attachment 192422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192422&action=review Oops, overlooked the test colors. Please fix to make it obvious what is pass and fail. > LayoutTests/svg/custom/use-modify-target-style-property-expected.svg:3 > +<circle cx="50" cy="50" r="50" style="fill:blue;" id="circle" /> The "good" color should be green. > LayoutTests/svg/custom/use-modify-target-style-property.svg:12 > + c.style.fill= "blue"; Change it to green. > LayoutTests/svg/custom/use-modify-target-style-property.svg:17 > +<circle cx="50" cy="50" r="50" style="fill:green;" id="circle" /> Start it red.
Takashi Sakamoto
Comment 6 2013-03-12 01:55:59 PDT
Created attachment 192671 [details] Patch
Takashi Sakamoto
Comment 7 2013-03-12 01:57:09 PDT
Comment on attachment 192422 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192422&action=review Thank you for reviewing. >> Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:31 >> +#include "SVGElementInstance.h" > > This needs to be protected with #if ENABLE(SVG) Done. >> Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp:403 >> + SVGElementInstance::invalidateAllInstancesOfElement(toSVGElement(m_parentElement), false); > > This also needs #if ENABLE(SVG) protection. Done. >> LayoutTests/svg/custom/use-modify-target-style-property-expected.svg:3 >> +<circle cx="50" cy="50" r="50" style="fill:blue;" id="circle" /> > > The "good" color should be green. Done. >> LayoutTests/svg/custom/use-modify-target-style-property.svg:12 >> + c.style.fill= "blue"; > > Change it to green. Done. >> LayoutTests/svg/custom/use-modify-target-style-property.svg:17 >> +<circle cx="50" cy="50" r="50" style="fill:green;" id="circle" /> > > Start it red. Done.
Stephen Chenney
Comment 8 2013-03-12 05:09:46 PDT
Comment on attachment 192671 [details] Patch Thanks. R=me.
Philip Rogers
Comment 9 2013-04-02 13:28:41 PDT
It looks like this was a good change but it never ended up landing. Takashi, can you update this patch and land it?
Ahmad Saleem
Comment 10 2022-05-29 08:07:44 PDT
I am still able to reproduce this bug using Safari 15.4 and Safari TP 146 based on attached Test case. Thanks!
Radar WebKit Bug Importer
Comment 11 2022-07-15 16:09:40 PDT
<rdar://problem/97099415>
Ahmad Saleem
Comment 12 2024-02-15 16:53:49 PST
We might have to do changes here: https://searchfox.org/wubkat/rev/c40451f6052e2805fb1c6abfb61fa322c67caf5b/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp#443 I tried: if (m_parentElement->isSVGElement()) invalidateInstances(); and if (m_parentElement->isSVGElement()) SVGElement::invalidateInstances(); __ But get compiler error - invalidateInstances' is a private member of 'WebCore::SVGElement' Added `SVGElement.h` header as well.
Ahmad Saleem
Comment 13 2024-02-15 17:51:36 PST
(In reply to Ahmad Saleem from comment #12) > We might have to do changes here: > > https://searchfox.org/wubkat/rev/c40451f6052e2805fb1c6abfb61fa322c67caf5b/ > Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp#443 > > I tried: > > if (m_parentElement->isSVGElement()) > invalidateInstances(); > > and > > if (m_parentElement->isSVGElement()) > SVGElement::invalidateInstances(); > > __ > > But get compiler error - invalidateInstances' is a private member of > 'WebCore::SVGElement' > > Added `SVGElement.h` header as well. Tried this as well: auto invalidateSVGInstance = SVGElement::invalidateInstances(); if (m_parentElement->isSVGElement()) invalidateSVGInstance; ____ but still got error about `private` member. If I move `void invalidateInstances()` to even Public, it still not work. :-(
Ahmad Saleem
Comment 14 2025-10-16 17:42:31 PDT
It seems to work fine now in Safari Technology Preview 230 (Tahoe).
Note You need to log in before you can comment on or make changes to this bug.