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

<canvas> width/height setters #3362

Open
annevk opened this issue Jan 16, 2018 · 2 comments
Open

<canvas> width/height setters #3362

annevk opened this issue Jan 16, 2018 · 2 comments
Labels
topic: canvas topic: reflect For issues with reflected IDL attributes and friends.

Comments

@annevk
Copy link
Member

annevk commented Jan 16, 2018

When setting the value of the width or height attribute, if the context mode of the canvas element is set to placeholder, the user agent must throw an "InvalidStateError" DOMException and leave the attribute's value unchanged.

The problem here is this is referencing the content attributes. The IDL attributes, for which we should be throwing the exception, are defined as follows:

The width and height IDL attributes must reflect the respective content attributes of the same name, with the same defaults.

This bug was introduced in 5a51d3e and quite a few of us overlooked it.

I wonder how to best solve this without copy-and-pasting part of the "reflect" algorithms for unsigned long. Should I give those algorithms specifically names and break somewhat from the paragraph-flow going on in that section so I can invoke them from here? Should we have "reflect with a custom setter" that can delegate to the "reflect setter" or some such?

cc @domenic @junov

@domenic
Copy link
Member

domenic commented Jan 24, 2018

I guess among the options of duplication, refactoring, and a hook into the reflection setter... I tend toward the hook into the reflection setter? I'm trying to think ahead to how it would work with #3238, and it still seems doable; we'd have e.g. [ReflectWithValidation] and define the validation in prose / implementations would define it in a well-known method.

So concretely I'd say something like "A reflecting IDL attribute may have a setter validation algorithm, which is run at various times throughout the below algorithms. If specified, it takes the new value being set (before any processing) and can throw an exception". Then, you can modify all the reflection algorithms (or just unsigned long??) like

On setting, first [validate] the new value. If the new value is in the range 0 to 2147483647, then let n be the new value, otherwise let n be the default value, or 0 if there is no default value; then, n must be converted to the shortest possible string representing the number as a valid non-negative integer and that string must be used as the new content attribute value.


Alternately I'd be fine with the refactoring solution; the above feels a bit over-engineered when I write it out.


I'd also be fine changing the structure of the reflection section to be more structured/less paragraphy, with individual algorithms for each getter/setter. Perhaps in a <dl>. That's fairly ambitious, but would help with #3238.

@annevk annevk added the topic: reflect For issues with reflected IDL attributes and friends. label Mar 3, 2023
@annevk
Copy link
Member Author

annevk commented Mar 3, 2023

cc @whatwg/canvas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: canvas topic: reflect For issues with reflected IDL attributes and friends.
Development

No branches or pull requests

2 participants