1621174 - The error handling of setting pseudoElement doesn't match the spec

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect, P3)

Description

[Boris Chiou [:boris]]

We have to fix the error type for syntactically invalid pseudos and accept unsupported (but syntactically valid) pseudos when setting pseudoElement, based on the spec discussion (https://github.com/w3c/csswg-drafts/issues/4745#issuecomment-596834928). And have to update the incorrect wpt for this.

Comment 1

[Boris Chiou [:boris]]

Attachment: Bug 1621174 - Fix error handling for setting KeyframeEffect.pseudoElement.

We should

1. throw a DOMException SyntaxError when setting pseudoElement a
   syntactically invalid string, and
2. accept it but no-op when setting
   pseudoElement a syntactically valid but non-supported pseudo.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bdf66b9a75e6
Fix error handling for setting KeyframeEffect.pseudoElement. r=birtles,smaug

Comment 3

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22207 for changes under testing/web-platform/tests

Comment 4

[Luke]

As per https://github.com/w3c/csswg-drafts/issues/4745#issuecomment-598001926, should this patch be throwing a SyntaxError when IsSupportedPseudoForWebAnimation is not satisfied?

Comment 5

[Brian Birtles (:birtles, Feb 26~ parental leave)]

(In reply to Luke from comment #4)

As per https://github.com/w3c/csswg-drafts/issues/4745#issuecomment-598001926, should this patch be throwing a SyntaxError when IsSupportedPseudoForWebAnimation is not satisfied?

Sorry, I meant to answer no to that first question there. Too many double negatives 😓.

Comment 6

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 7

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/bdf66b9a75e6

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 9

[Boris Chiou [:boris]]

Comment on attachment 9132413 [details]
Bug 1621174 - Fix error handling for setting KeyframeEffect.pseudoElement.

Beta/Release Uplift Approval Request

• User impact if declined: This patch fixes the error handle of the setter of pseudo-selector on web-animations, which has been just shipped already. It'd be great if we could land this patch to match the current spec. Besides, we drop two redundant dom error messages which were added on Firefox 75, so this might save localizers from having to localize these strings (if there are any in dom/bindings/Errors.msg).
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only change the error type for web animations
• String changes made/needed: We drop two dom error messages.

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 9132413 [details]
Bug 1621174 - Fix error handling for setting KeyframeEffect.pseudoElement.

replacing some error types for animations for better spec compliance, approved for 75.0b4

Comment 11

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/96d3f71da2dd