NEW322974
SVGTransformList re-parse detaches the item at an index whose transform function changed, and every item after it
https://bugs.webkit.org/show_bug.cgi?id=322974
Summary SVGTransformList re-parse detaches the item at an index whose transform funct...
Karl Dubost
Reported 2026-08-31 01:55:18 PDT
SVGTransformList updates its items in place when the new attribute has the same transform function at that index, and throws them away when it does not. So translate(...) becoming scale(...) at index 0 detaches the SVGTransform a script is holding, and because the code drops out of in-place mode at the first mismatch, it also discards every item after that index even where those would have matched. Firefox keeps the objects in both cases. ### Measured, 3 engines, 2026-08-29 Probe attached, group T. All 7 controls pass in all three runs, no errors. Safari 27.0 Firefox 156.0 Chrome 154 translate(1,2) -> scale(3), same count held item still at index 0 no YES no held item reports the new type no YES no held item's matrix reports the new components, and is the same object no YES no translate(1,2) scale(3) -> scale(4) translate(5,6) both held items survive no YES no Firefox is the only engine that gets this right. Safari's detail on the type row is "held.type=2 expected=3 list0.type=3": the list itself is correct and the object the script holds was left behind as a translate. Safari gets the SAME-function case right on all branches (same count, grow, and the survivors of a shrink), which is why this reads as a narrowing of an otherwise correct implementation rather than as a list that was never done. Chrome fails the whole file for these lists, and separately returns a fresh wrapper from every getItem call, so identity there is not observable at all. ### Watch out when writing the test Safari's matrix identity check PASSES: after the change, held.matrix === the object the script was holding. What fails is the components, m.a is 1 where the spec requires 3, because the detached item kept its own matrix. Asserting matrix identity alone reports success. ### Testcase <svg xmlns="http://www.w3.org/2000/svg"> <g id="g" transform="translate(1,2)"></g> </svg> const list = g.transform.baseVal; const held = list.getItem(0); const m = held.matrix; g.setAttribute("transform", "scale(3)"); // same item count, different function list.getItem(0) === held; // true per spec. Safari says false. held.type; // 3 (SVG_TRANSFORM_SCALE) per spec. Safari says 2 (TRANSLATE). m.a; // 3 per spec. Safari says 1. And the reordering case, which loses two items to one mismatch: g.setAttribute("transform", "translate(1,2) scale(3)"); const h0 = list.getItem(0), h1 = list.getItem(1); g.setAttribute("transform", "scale(4) translate(5,6)"); list.getItem(0) === h0; // true per spec. Safari says false. list.getItem(1) === h1; // true per spec. Safari says false. gradientTransform and patternTransform behave the same way, so this is not about transform being a presentation attribute. ### Spec https://w3c.github.io/svgwg/svg2-draft/types.html#TermSynchronizeList Step 4.1 detaches only "each object in the list at an index greater than or equal to new length", and one item becoming one item does not reach it. Step 4.4 then runs over every surviving index with no condition on the item's current transform function: 4.4.1 "Let item be the object in the list at index index." 4.4.2 "Let v be the value in value at index index." 4.4.3 "Set item's value to v." 4.4.4 "If item is an SVGTransform object, then set the components of its matrix object to match the new transform function value." 4.4.4 exists precisely for this case and says to set the components of its matrix object, not to replace the matrix and not to leave it alone. Same statement from the other direction: "An SVGTransform object's matrix object is always kept synchronized with its value", https://w3c.github.io/svgwg/svg2-draft/coords.html#TransformMatrixObject ### Cause https://github.com/WebKit/WebKit/blob/4ae53427d5d8e52791da2cfe0c0c98b877b2b5d5/Source/WebCore/svg/SVGTransformList.cpp#L136-L149 if (currentListReplacement == ListReplacement::Replace && itemIndex < m_items.size() && parsedTransformType == m_items[itemIndex]->type()) { if (!parseAndReplaceTransform(*parsedTransformType, buffer, m_items[itemIndex])) return false; } else { // Switch to `Append` mode and remove the existing SVGTransforms starting from `itemIndex`. if (currentListReplacement == ListReplacement::Replace) { currentListReplacement = ListReplacement::Append; resize(itemIndex); } parsedTransformType == m_items[itemIndex]->type() is the whole thing. The resize(itemIndex) in the else branch is what turns one mismatch into a loss of every later item. The in-place path came from 292545@main (bug 290192) and 294342@main (bug 292303), both allocation-churn patches, so the type equality was a sufficient condition for the case they cared about (an animation rewriting the same function with new numbers) rather than a rule about when an item may survive. Changing an item's type in place already works, because every SVGTransformValue setter sets the type and writes through the same matrix object. Source/WebCore/svg/SVGTransformValue.h: void setScale(float sx, float sy) { m_type = SVG_TRANSFORM_SCALE; m_angle = 0; m_rotationCenter = { }; m_matrix->value() = AffineTransform::makeScale({ sx, sy }); } m_matrix is the object the script holds through SVGTransform.matrix, so 4.4.4 and the [SameObject] on matrix both fall out for free. The change is to drop the type equality from the condition, drop the matching ASSERT in parseAndReplaceTransform, and pass the parsed type in rather than reading it off the item: https://github.com/WebKit/WebKit/blob/4ae53427d5d8e52791da2cfe0c0c98b877b2b5d5/Source/WebCore/svg/SVGTransformListInlines.h#L94-L96 That also removes a per-frame allocation from the case where an animation switches function, which is the same argument bug 290192 was landed on. ### Tests No WPT coverage. svg/coordinate-systems/SVGTransformList.list-01.html is the closest and cannot distinguish: its only setAttribute is inside the setup helper, before anything is held. No WebKit-local test asserts either answer for this case, so there is nothing to rebaseline. ### Related - Bug 314992 asks for the in-place path to be removed. Answered in comment #2 there: the algorithm requires in-place, and removing it would take WebKit from 1-of-4 lists correct to 0-of-4. This bug is the other half of that answer: the in-place path does not go far enough. - The same defect on the other three list interfaces, filed separately.
Attachments
testcase-probe.html (5.43 KB, text/html)
2026-08-31 02:15 PDT, Karl Dubost
no flags
Karl Dubost
Comment 1 2026-08-31 02:15:56 PDT
Created attachment 481222 [details] testcase-probe.html
Radar WebKit Bug Importer
Comment 2 2026-09-07 01:56:11 PDT
Karl Dubost
Comment 3 2026-10-05 19:47:16 PDT
Note You need to log in before you can comment on or make changes to this bug.