261656 2023-09-17 12:39:28 -0700 [SVG2] Allow leading and trailing whitespace in svg attributes using <integer>, <angle>, <number>, <length> and <percentage> 2025-11-27 17:00:12 -0800 1 1 1 Unclassified WebKit SVG WebKit Nightly Build Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=267873 https://bugs.webkit.org/show_bug.cgi?id=303218 BrowserCompat, InRadar P2 Normal --- 267560 1 ahmad.saleem792 ahmad.saleem792 cdumez heycam karlcow rbuis sabouhallawa simon.fraser webkit-bug-importer zakr zimmermann oldest_to_newest 1977933 0 ahmad.saleem792 2023-09-17 12:39:28 -0700 Hi Team, This is another Blink's commit, where we are failing test cases: Blink Commit: https://src.chromium.org/viewvc/blink?view=revision&revision=175785 Test Case (svg/parser/whitespace-number.html) - https://jsfiddle.net/34nqh1uy/show Test Case (svg/parser/whitespace-length.html) - https://jsfiddle.net/34nqh1uy/1/show and there are others as well. ^ We are failing lot of these tests on WebKit ToT as well. Chrome Canary 119 and Firefox Nightly 119 both pass these test case. Just wanted to raise so we can track it. Thanks! 1978606 1 karlcow 2023-09-20 02:30:28 -0700 The test: data:text/html,<svg id="testcontainer"><defs><marker></marker><stop></stop><filter><feTurbulence></feTurbulence></filter></defs></svg> document.getElementsByTagName('stop')[0].setAttribute('offset', '-47a') Then they try to document.getElementsByTagName('stop')[0].getAttribute('offset') which returns '-47a' in all browsers. Where it differs is when requesting: document.getElementsByTagName('stop')[0].offset.baseVal it returns -47 in Safari 0 in Firefox 0 in Chrome It seems Safari is normalizing the value by ignoring the trailing garbage. While the others are saying bad values let's reset it. In fact document.getElementsByTagName('stop')[0].offset returns on Safari. {animVal: -47, baseVal: -47} https://svgwg.org/svg2-draft/pservers.html#GradientStopAttributes the initial value is indeed 0. The values <number> A number usually ranging from 0 to 1. <percentage> A percentage usually ranging from 0% to 100%. I don't remember where are the processing rules for the invalid values in CSS. But baseVal and animVal are defined in https://svgwg.org/svg2-draft/types.html#__svg__SVGAnimatedString__baseVal 1979246 2 karlcow 2023-09-21 19:15:11 -0700 Related(?) to https://searchfox.org/wubkat/rev/f997a9edb80cd79caeb1d0a1b87610ffd7a56e88/Source/WebCore/svg/SVGStopElement.cpp#53-63 It converts to float but doesn't to check if the input value if the value looks like a float before converting it to a float. The patch from Chromium contains a parseNumber https://src.chromium.org/viewvc/blink/trunk/Source/core/svg/SVGParserUtilities.cpp?annotate=175785&pathrev=175785#l230 I wonder if it exists in WebKit? 1979247 3 karlcow 2023-09-21 19:16:29 -0700 Maybe around https://searchfox.org/wubkat/source/Source/WebCore/svg/SVGParserUtilities.cpp#142 1979252 4 karlcow 2023-09-21 19:38:43 -0700 So Chris Dumez helped me understand better WTF_EXPORT_PRIVATE float toFloat(bool* ok = nullptr) const; So we could do something like bool success = false; float result = attribute.toFloat(&success); if (success) // use result. else // ... 1979265 5 karlcow 2023-09-21 21:27:35 -0700 Ahmad, I think I have a patch for this. It needs a bit more work but It's passing a lot more tests. 1979268 6 karlcow 2023-09-21 22:16:16 -0700 OK My patch gets all green for Test Case (svg/parser/whitespace-number.html) - https://jsfiddle.net/34nqh1uy/show Now let's see the other. :) Test Case (svg/parser/whitespace-length.html) - https://jsfiddle.net/34nqh1uy/1/show 1979656 7 webkit-bug-importer 2023-09-24 12:40:14 -0700 <rdar://problem/115963075> 2037867 8 ahmad.saleem792 2024-05-25 06:11:59 -0700 Blink Commit - https://github.com/chromium/chromium/commit/77c8625da85bd956992b81233d451b87c39e7dea 2052393 9 ahmad.saleem792 2024-08-13 10:19:39 -0700 > Source/WebCore/svg/SVGTransformable.cpp: From: auto parsedNumber = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); To: auto parsedNumber = parseNumber(buffer, SVGWhitespaceMode::DisallowWhitespace); and From: auto parsedNumber = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); To: auto parsedNumber = parseNumber(buffer, SVGWhitespaceMode::DisallowWhitespace); ___ > Source/WebCore/svg/SVGPointList.cpp: From: auto yPos = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); To: auto yPos = parseNumber(buffer, SVGWhitespaceMode::DisallowWhitespace); ___ > Source/WebCore/svg/SVGLengthValue.cpp: From: auto convertedNumber = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); To: auto convertedNumber = parseNumber(buffer, SVGWhitespaceMode::AllowLeadingWhitespace); ___ > Source/WebCore/svg/SVGFitToViewBox.cpp: From: auto height = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); To: auto height = parseNumber(buffer, SVGWhitespaceMode::AllowLeadingAndTrailingWhitespace); ___ > Source/WebCore/svg/SVGAnimationElement.cpp: From: auto posD = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); To: auto posD = parseNumber(buffer, SVGWhitespaceMode::DisallowWhitespace); ____ > Source/WebCore/svg/SVGAngleValue.cpp: From: auto valueInSpecifiedUnits = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); To: uto valueInSpecifiedUnits = parseNumber(buffer, SVGWhitespaceMode::AllowLeadingWhitespace); ___ > Source/WebCore/svg/SVGParserUtilities.h: Delete below: enum class SuffixSkippingPolicy { DontSkip, Skip }; and add this: enum class SVGWhitespaceMode { DisallowWhitespace = 0, AllowLeadingWhitespace = 0x1, AllowTrailingWhitespace = 0x2, AllowLeadingAndTrailingWhitespace = AllowLeadingWhitespace | AllowTrailingWhitespace }; Change: std::optional<float> parseNumber(StringParsingBuffer<LChar>&, SuffixSkippingPolicy = SuffixSkippingPolicy::Skip); std::optional<float> parseNumber(StringParsingBuffer<LChar>&, SVGWhitespaceMode mode = SVGWhitespaceMode::AllowLeadingAndTrailingWhitespace); and std::optional<float> parseNumber(StringParsingBuffer<UChar>&, SuffixSkippingPolicy = SuffixSkippingPolicy::Skip); std::optional<float> parseNumber(StringParsingBuffer<UChar>&, SVGWhitespaceMode mode = SVGWhitespaceMode::AllowLeadingAndTrailingWhitespace); and std::optional<float> parseNumber(StringView, SuffixSkippingPolicy = SuffixSkippingPolicy::Skip); std::optional<float> parseNumber(StringView, SVGWhitespaceMode mode = SVGWhitespaceMode::AllowLeadingAndTrailingWhitespace); ____ > Source/WebCore/svg/SVGParserUtilities.cpp: Function definition: template <typename CharacterType, typename FloatType = float> static std::optional<FloatType> genericParseNumber(StringParsingBuffer<CharacterType>& buffer, SuffixSkippingPolicy skip = SuffixSkippingPolicy::Skip) to: template <typename CharacterType, typename FloatType = float> static std::optional<FloatType> genericParseNumber(StringParsingBuffer<CharacterType>& buffer, SVGWhitespaceMode mode = SVGWhitespaceMode::AllowLeadingAndTrailingWhitespace) and in below: if (mode == SVGWhitespaceMode::AllowLeadingWhitespace) skipOptionalSVGSpaces(buffer); and if (mode == SVGWhitespaceMode::AllowTrailingWhitespace) skipOptionalSVGSpacesOrDelimiter(buffer); Another function definition: std::optional<float> parseNumber(StringView string, SuffixSkippingPolicy skip) to: std::optional<float> parseNumber(StringView string, SVGWhitespaceMode mode) and return readCharactersForParsing(string, [skip](auto buffer) -> std::optional<float> { auto result = genericParseNumber(buffer, skip); to: return readCharactersForParsing(string, [mode](auto buffer) -> std::optional<float> { auto result = genericParseNumber(buffer, mode); Another function definition: std::optional<float> parseNumber(StringParsingBuffer<LChar>& buffer, SuffixSkippingPolicy skip) to: std::optional<float> parseNumber(StringParsingBuffer<LChar>& buffer, SVGWhitespaceMode mode) and change return to: return genericParseNumber(buffer, mode); Another function definition: std::optional<float> parseNumber(StringParsingBuffer<UChar>& buffer, SuffixSkippingPolicy skip) to: std::optional<float> parseNumber(StringParsingBuffer<UChar>& buffer, SVGWhitespaceMode mode) and change return: return genericParseNumber(buffer, mode); ___ > Source/WebCore/svg/SVGParserUtilities.cpp: parseNumberOptionalNumber: auto y = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); to auto y = parseNumber(buffer, SVGWhitespaceMode::AllowLeadingAndTrailingWhitespace); parseRect: auto height = parseNumber(buffer, SuffixSkippingPolicy::DontSkip); to auto height = parseNumber(buffer, SVGWhitespaceMode::DisallowWhitespace); ___ It does progress test cases but not where we have have scientific notations. 2052601 10 karlcow 2024-08-13 19:26:46 -0700 Ahmad, I think scientific notations are still not yet a spec thing. 2052602 11 karlcow 2024-08-13 19:33:21 -0700 If there are WPT tests relying on scientific notations they should be removed. A bit like it was done in https://github.com/web-platform-tests/wpt/commit/06e103ad6136bf0988f9d1614db05d8a652e0570 2112049 12 ahmad.saleem792 2025-04-20 04:26:13 -0700 Syncing some passing tests here - https://github.com/WebKit/WebKit/pull/44306 At least, it would give some coverage while we try to progress remaining cases. I do have patch to make some progress more without any regression, so I think eve n partial progression is better than nothing. So might end-up doing PR later. 2112149 13 ahmad.saleem792 2025-04-21 04:22:01 -0700 Pull request: https://github.com/WebKit/WebKit/pull/44323 2112150 14 ahmad.saleem792 2025-04-21 04:23:21 -0700 (In reply to Ahmad Saleem from comment #13) > Pull request: https://github.com/WebKit/WebKit/pull/44323 NOTE - I am changing it to to draft, it still progress test cases without regression: https://jsfiddle.net/maseh9v7/ -> whitespace-angle-2.html (105 -> 156) https://jsfiddle.net/pad9c3mo/ -> whitespace-angle-1.html (108 -> 288) https://jsfiddle.net/2639xdzs/ -> whitespace-integer.html (27 - no change) https://jsfiddle.net/6bf32qhk/ -> whitespace-number.html (1074 - no change) https://jsfiddle.net/2b43fprz/ -> whitespace-length.html (704 -> 942) Plus it is partial merge, I will try to do remaining bit and hopefully can progress it fully but either way - even landing as is for partial progression like above wouldn't be so bad.