Bug 269477
| Summary: | auto-start-reverse WPT is failing | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Karl Dubost <karlcow> |
| Component: | SVG | Assignee: | Karl Dubost <karlcow> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | ahmad.saleem792, sabouhallawa, webkit-bug-importer, zimmermann |
| Priority: | P2 | Keywords: | InRadar |
| Version: | Safari 17 | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| URL: | https://svgwg.org/svg2-draft/painting.html#InterfaceSVGMarkerElement | ||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=138456 | ||
| Bug Depends on: | 138456 | ||
| Bug Blocks: | |||
Karl Dubost
In expose auto-start-reverse enum value
https://bugzilla.mozilla.org/show_bug.cgi?id=1685543
related to https://github.com/w3c/svgwg/issues/424
> RESOLVED: Where SVG 2 has added a new attribute value for an attribute that has an existing DOM enumeration, we will add matching new enumeration values
https://github.com/w3c/svgwg/issues/424#issuecomment-387176205
The spec is:
https://svgwg.org/svg2-draft/painting.html#InterfaceSVGMarkerElement
Currently Safari fails:
https://wpt.fyi/results/svg/types/scripted/SVGAnimatedEnumeration-SVGMarkerElement.html
What the test is doing.
function createMarker() {
let markerElement = document.createElementNS("http://www.w3.org/2000/svg", "marker");
markerElement.setAttribute("markerUnits", "userSpaceOnUse");
markerElement.setAttribute("orient", "auto");
return markerElement;
}
let markerElement = createMarker();
// Switch to 'auto-start-reverse' value - by modifying orientType.
markerElement.orientType.baseVal = SVGMarkerElement.SVG_MARKER_ORIENT_AUTO_START_REVERSE;
assert_equals(markerElement.orientAngle.baseVal.value, 0);
assert_equals(markerElement.orientAngle.baseVal.unitType, SVGAngle.SVG_ANGLETYPE_UNSPECIFIED);
assert_equals(markerElement.orientType.baseVal, SVGMarkerElement.SVG_MARKER_ORIENT_AUTO_START_REVERSE);
assert_equals(markerElement.getAttribute('orient'), "auto-start-reverse");
}, "Test auto-start-reverse");
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Karl Dubost
but it seems the spec has not been updated yet?!
I don't see SVG_MARKER_ORIENT_AUTO_START_REVERSE
Ahmad Saleem
Some WPTs:
> marker-orient-001.svg
> SVGAnimatedEnumeration-SVGMarkerElement.html
> animate-marker-orient-from-auto-to-auto-start-reverse.html
___
NOTE - from `See Also` - we do support it but I think we don't have all the hooks configured properly or something else. Still need to investigate but adding information here as note keeping.
Karl Dubost
The goal is to add
const unsigned short SVG_MARKER_ORIENT_AUTO_START_REVERSE = 3;
to the idl file
https://searchfox.org/wubkat/source/Source/WebCore/svg/SVGMarkerElement.idl
The equivalent on Gecko
https://searchfox.org/mozilla-central/source/dom/webidl/SVGMarkerElement.webidl
But to make sure to not break anything too. So probably a couple of modifications in the process.
Once it is done, all the tests from
https://wpt.fyi/results/svg/types/scripted/SVGAnimatedEnumeration-SVGMarkerElement.html
will pass.
Ahmad Saleem
(In reply to Karl Dubost from comment #3)
> The goal is to add
> const unsigned short SVG_MARKER_ORIENT_AUTO_START_REVERSE = 3;
> to the idl file
> https://searchfox.org/wubkat/source/Source/WebCore/svg/SVGMarkerElement.idl
>
> The equivalent on Gecko
> https://searchfox.org/mozilla-central/source/dom/webidl/SVGMarkerElement.
> webidl
>
> But to make sure to not break anything too. So probably a couple of
> modifications in the process.
>
> Once it is done, all the tests from
> https://wpt.fyi/results/svg/types/scripted/SVGAnimatedEnumeration-
> SVGMarkerElement.html
>
> will pass.
Tried this:
> Source/WebCore/svg/SVGMarkerElement.idl:
const unsigned short SVG_MARKER_ORIENT_AUTO_START_REVERSE = 3;
> Source/WebCore/svg/SVGMarkerTypes.h (in enum - SVGMarkerOrientType):
Change:
SVGMarkerOrientAutoStartReverse = SVGMarkerOrientUnknown
to:
SVGMarkerOrientAutoStartReverse
> Source/WebCore/svg/SVGMarkerElement.h (in enum):
SVG_MARKER_ORIENT_AUTO_START_REVERSE = SVGMarkerOrientAutoStartReverse
and (this might not be required):
void setOrientToAutoReverseAngle(const SVGAngle&);
> Source/WebCore/svg/SVGMarkerElement.cpp:
void SVGMarkerElement::setOrientToAutoReverseAngle(const SVGAngle& autoreverseangle)
{
Ref { m_orientAngle } -> baseVal()->newValueSpecifiedUnits(autoreverseangle.unitType(), autoreverseangle.valueInSpecifiedUnits());
invalidateMarkerResource();
}
(Similar to setOrientToAuto) - but need fixing.
___
This compiles but don't progress anything and get 'Type Error'.
Just initial start.
Karl Dubost
Ahmad,
This probably needs to be changed too.
https://searchfox.org/wubkat/rev/c40451f6052e2805fb1c6abfb61fa322c67caf5b/Source/WebCore/svg/SVGMarkerTypes.h#40-48
Radar WebKit Bug Importer
<rdar://problem/123453058>
Karl Dubost
Pull request: https://github.com/WebKit/WebKit/pull/49526
EWS
Committed 299042@main (ced6588f9f23): <https://commits.webkit.org/299042@main>
Reviewed commits have been landed. Closing PR #49526 and removing active labels.