| Summary: | SVGLengthValue.cpp should support rem (probably other) units for width and height attributes | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Karl Dubost <karlcow> |
| Component: | SVG | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW --- | ||
| Severity: | Normal | CC: | ahmad.saleem792, sabouhallawa, webkit-bug-importer, zimmermann |
| Priority: | P2 | Keywords: | BrowserCompat, InRadar |
| Version: | Safari 17 | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | |||
| Bug Blocks: | 253937 | ||
|
Description
Karl Dubost
2023-06-12 21:56:38 PDT
SeeAlso https://svgwg.org/svg2-draft/geometry.html#Sizing https://svgwg.org/svg2-draft/styling.html#PresentationAttributes https://svgwg.org/svg2-draft/types.html#presentation-attribute-css-value This is probably the cause of a webcompat issue on https://www.usnews.com/best-colleges/franklin-w-olin-college-of-engineering-39463 when scrolling up and when the menu is trying to set a right chevron. The issue is happening also for Firefox. # For em values Starting with data:text/html,<svg></svg> > document.querySelector('svg').width.baseVal < SVGLength {unitType: 2, value: 300, valueInSpecifiedUnits: 100, valueAsString: "100%", newValueSpecifiedUnits: function, …} > document.querySelector('svg').setAttribute('width', '2em') < undefined > document.querySelector('svg').width.baseVal < SVGLength {unitType: 3, value: 32, valueInSpecifiedUnits: 2, valueAsString: "2em", newValueSpecifiedUnits: function, …} # For rem values Restarting again with data:text/html,<svg></svg> > document.querySelector('svg').width.baseVal < SVGLength {unitType: 2, value: 300, valueInSpecifiedUnits: 100, valueAsString: "100%", newValueSpecifiedUnits: function, …} > document.querySelector('svg').setAttribute('width', '2rem') [Error] Error: Invalid value for <svg> attribute width="2rem" Console Evaluation (Console Evaluation 2:3) < undefined > document.querySelector('svg').width.baseVal < SVGLength = $1 unitType: 2 value: 32 valueAsString: "100%" valueInSpecifiedUnits: 100 SVGLength Prototype This is in fact here https://searchfox.org/wubkat/source/Source/WebCore/svg/SVGLengthValue.cpp#65 rem is missing. Starting with data:text/html,<svg%20width="50"></svg>
We get for document.querySelector('svg').width.baseVal
unitType: 1
value: 50
valueAsString: "50"
valueInSpecifiedUnits: 50
Let's set to 3 rem.
document.querySelector('svg').setAttribute('width', '3rem')
unitType: 2 (SVG_LENGTHTYPE_PERCENTAGE)
value: 48
valueAsString: "100%"
valueInSpecifiedUnits: 100
Why the 48 here?
It reports the issue
reportAttributeParsingError(parseError, name, newValue);
and then goes on with:
SVGFitToViewBox::parseAttribute(name, newValue);
SVGZoomAndPan::parseAttribute(name, newValue);
SVGGraphicsElement::attributeChanged(name, oldValue, newValue, attributeModificationReason);
We do have WPT here: https://wpt.fyi/results/svg/types/scripted/SVGLength-rem.html?label=master&label=experimental&aligned=&q=safari%3Afail Or this bug is different? (In reply to Ahmad Saleem from comment #8) > Blink Commit - > https://src.chromium.org/viewvc/blink?view=revision&revision=193355 Based on this (inspired), I have local patch, which does progress sub-test of WPT but I think we have hardcoded font-size, so one sub-test continue to fail. I think something is better than nothing, so I am happy to do PR but keeping this as master bug and separate for each length type. I wonder if it would fix also assertions in http://wpt.live/svg/types/scripted/SVGLength.html there are a lot of things failing there, which seems basic. (In reply to Karl Dubost from comment #10) > I wonder if it would fix also assertions in > http://wpt.live/svg/types/scripted/SVGLength.html > there are a lot of things failing there, which seems basic. I tried 'IDL' file changes and we start to progress few more tests but also start failing few of current tests in `svg/dom/'. So I never did the change. (In reply to Ahmad Saleem from comment #11) > (In reply to Karl Dubost from comment #10) > > I wonder if it would fix also assertions in > > http://wpt.live/svg/types/scripted/SVGLength.html > > there are a lot of things failing there, which seems basic. > > I tried 'IDL' file changes and we start to progress few more tests but also > start failing few of current tests in `svg/dom/'. So I never did the change. Plus - I also have `rem` patch locally - just want to land few of current PR and reduce my queue before doing PR to add `rem` support. :-) (In reply to Ahmad Saleem from comment #11) > (In reply to Karl Dubost from comment #10) > > I wonder if it would fix also assertions in > > http://wpt.live/svg/types/scripted/SVGLength.html > > there are a lot of things failing there, which seems basic. > > I tried 'IDL' file changes and we start to progress few more tests but also > start failing few of current tests in `svg/dom/'. So I never did the change. From SVGLength.idl, we have `unrestricted` while as per spec, we shouldn't. This is also creating a series of error messages in the console for https://vimeo.com/channels/staffpicks [Error] Error: Invalid value for <svg> attribute width="1.5rem" setValueForProperty (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:2:20315) _updateDOMProperties (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:25153) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:21494) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) mountChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:19271) _createInitialChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:23382) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:21543) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) mountChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:19271) _createInitialChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:23382) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:21543) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) mountChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:19271) _createInitialChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:23382) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:21543) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) mountChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:19271) _createInitialChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:23382) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:21543) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) mountChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:19271) _createInitialChildren (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:23382) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:21543) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) performInitialMount (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:10790) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:3:9681) mountComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:22674) a (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:14516) perform (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:5:13022) s (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:14730) perform (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:5:13022) _renderNewRootComponent (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:15998) _renderSubtreeIntoContainer (react_prod_combined_ccb7d5c4cff728ceadb9996f385b2d09.min.js:4:17004) d (top_navigation.2644aa259e2ca7a1053f.bundle.min.js:54:157882) (anonymous function) (top_navigation.2644aa259e2ca7a1053f.bundle.min.js:28:565671) (anonymous function) (top_navigation.2644aa259e2ca7a1053f.bundle.min.js:28:565691) n (top_navigation.2644aa259e2ca7a1053f.bundle.min.js:1:115) (anonymous function) (top_navigation.2644aa259e2ca7a1053f.bundle.min.js:1:453) Global Code (top_navigation.2644aa259e2ca7a1053f.bundle.min.js:1:463) |