Closed Bug 1621174 Opened 5 years ago Closed 5 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla76
Tracking Status
firefox75 --- fixed
firefox76 --- fixed

People

(Reporter: boris, Assigned: boris)

References

Details

Attachments

(1 file)

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: nobody → boris.chiou

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.
Pushed by bchiou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bdf66b9a75e6 Fix error handling for setting KeyframeEffect.pseudoElement. r=birtles,smaug
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/22207 for changes under testing/web-platform/tests

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

(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 😓.

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla76
Upstream PR merged by moz-wptsync-bot

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.
Attachment #9132413 - Flags: approval-mozilla-beta?

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

Attachment #9132413 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: