Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add a
lang
IDL Attribute to CanvasTextDrawingStyles, and clarify "direction" on same #10873base: main
Are you sure you want to change the base?
Add a
lang
IDL Attribute to CanvasTextDrawingStyles, and clarify "direction" on same #10873Changes from all commits
eba506c
caa22ca
488b95b
01e6185
b6df5ee
1b5a1df
152e3a4
60a5971
720ff90
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple issues here:
letterSpacing
does.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I did "2" as you meant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to manipulate an internal concept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean here. Has the last set of edits addressed this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. If you look at the
letterSpacing
getter it is defined as follows:"letter spacing" there is the internal concept. That is what's missing here. The internal concept can have the same name as the getter, but it is distinct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we're talking about the setter here. I feel I'm being slow, but is this sort of what you mean (sans links):
The lang setter steps are to set the value and update the internal font language
I think we agree that the value in
lang
is the string that round-trips, while the internal font language really is internal and not visible to authors.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's about both the getter and the setter. If the internal font language is the concept you could rewrite the getter as:
(Note that you need to use "this" as well. We need to know where to find this internal concept after all.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The desired behavior for the getter relates to #10884. Do we want to return the "inherit" string or the internal font language? For
direction
Chromium and Webkit does the latter, while Firefox does the former.From a back-compat position it's probably very hard to change
direction
, so maybe we should speclang
anddirection
to do the Chromium/WebKit thing. That's probably what developers most want. But it means no round-tripping.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there is no interop, is there really a back-compat issue? Has Firefox received bug reports about this?
I am very interested in the results from your counters, but as a web-dev I was very surprised when you metionned Chrome and Safari's behavior for
direction
. I personnaly think it's bad to have the computed value instead of"inherit"
there. We don't have a granular way of resetting the context's attributes, so I wouldn't be surprised someone made their own thing by just reading all the props on the context and setting them back later. Returning the computed value would break that.For
lang
, as currently written,"inherit"
does not update outside of the setter, because the value stored is computed in the setter itself. So in that case it would make sense to return the stored (computed) value, but I think"inherit"
wouldn't be the proper keyword. If"inherit"
really did update even after setting, then returning"inherit"
would make more sense IMO.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will take a couple of months to get counter information, as far as I can tell. I'm not sure what it can tell us about intent or desire, so I'll also try to find some hints from various developer forums (like how many people have asked about this issue, or asked how to get the direction or something like that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, per https://w3ctag.github.io/design-principles/#attributes-like-data, it should roundtrip, if it doesn't it should be, a setter method, and a getter method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I tend to agree with @Kaiido as to what we want here. It seems very strange to return the computed value, especially if it's informed by layout. (To be clear, I know how it all works, just speaking from the perspective of HTML standard semantics.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably perform the same checks as in https://whatpr.org/html/10873/dom.html#the-lang-and-xml:lang-attributes so that it's limited to BCP 47 allowed values and the special inherit value (, and maybe
""
?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it says it must be valid, but then it says:
If the resulting value is not a recognized language tag, then it must be treated as an unknown language having the given language tag, distinct from all other languages. For the purposes of round-tripping or communicating with other services that expect language tags, user agents should pass unknown language tags through unmodified, and tagged as being BCP 47 language tags, so that subsequent services do not interpret the data as another type of language description. [[BCP47]](https://whatpr.org/html/10873/references.html#refsBCP47)
So maybe say it must be valid but the UA must still pass invalid values through unmodified?