The error handling of setting pseudoElement doesn't match the spec
Categories
(Core :: DOM: Animation, defect, P3)
Tracking
()
People
(Reporter: boris, Assigned: boris)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details |
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.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
We should
- throw a DOMException SyntaxError when setting pseudoElement a
syntactically invalid string, and - accept it but no-op when setting
pseudoElement a syntactically valid but non-supported pseudo.
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•5 years ago
|
||
(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 7•5 years ago
|
||
bugherder |
Assignee | ||
Comment 9•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
bugherder uplift |
Description
•