Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify which string value is used when setting SVGAnimatedString's baseVal #961

Open
fred-wang opened this issue Jan 6, 2025 · 2 comments

Comments

@fred-wang
Copy link
Contributor

The corresponding paragraph is https://svgwg.org/svg2-draft/single-page.html#types-InterfaceSVGAnimatedString

Trusted type integration was made in https://github.com/w3c/svgwg/pull/934/files but I can't see the changes in the live version.

As I see, SVGAnimatedString is currently in multiple places:

  • SVGURIReference's href (used by SVGScriptElement and many others)
  • SVGAElement's target
  • SVGElement's className

The steps for setting baseVal call "set the reflected attribute to value" which I believe corresponds to this algo which accepts a string. For SVGScriptElement, "value" is set to the result of Get Trusted Type compliant string. But for other cases, it's set to the "specified value" which can be a TrustedScriptURL. Probably we should be more explicit and say we extract data as in https://html.spec.whatwg.org/#tt-trustedhtml-data

Note that in the case of a TrustedScriptURL with a forged stringified, that means setting these properties (e.g. className) would behave differently from properties only accepting a string.

cc @lukewarlow

@fred-wang
Copy link
Contributor Author

Note that in the case of a TrustedScriptURL with a forged stringified, that means setting these properties (e.g. className) would behave differently from properties only accepting a string.

@lukewarlow pointed out that this probably not true. Forging the toString() won't make a difference, https://w3c.github.io/trusted-types/dist/spec/#trustedhtml-stringification-behavior will be used and so this will use the extracted data. I guess we should do the same for the ones using SVGAnimatedString

@fred-wang
Copy link
Contributor Author

@lukewarlow SVGScriptElement can have SVGAnimatedString properties other than href and actually I believe className is the only one.

As I read the current text, Get Trusted Type compliant string will be executed with the 'SVGScriptElement href' when one tries to set SVGScriptElement.className to a plain string, but we probably just want to use the specified value in that case.

I added more tests in to cover this (as well as href on non-script elements) where we don't want to run the TT checks: https://phabricator.services.mozilla.com/D233341. Luckily, Gecko implements SVGAnimatedString for className in a separate C++ class so it's easy to make the test pass, but I don't know whether the checking the reflected attribute name is straightforward in other engines...

Chromium fails for "Assign string to SVGScriptElement.className.baseVal.", so it indeed executes the trusted type check for class name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant