-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
#10873
base: main
Are you sure you want to change the base?
Conversation
Also clean up what "inherit" means for direction.
@AndresRPerez12, @annevk, @fserb, @Kaiido Note that currently Chromium does not resolve |
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.
Thanks for tackling this!
I personally think it's great this hidden inheritance is finally made explicit, and having a way to set it on the context directly is a big plus.
value "inherit".</p> | ||
|
||
<p>The default is "inherit", which uses the <span data-x="attr-lang">lang</span> of the | ||
<code>canvas</code> element, or the Document associated with a Worker when there is no |
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 suppose "Worker" is a typo here? Probably meant to say OffscreenCanvas
?
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.
Yes, that would be easier for developers to understand. I was basically using the final step in the algorithm to look it up, but that's an implementation thing.
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.
Is it though? Having the worker side have to look up for the attached document's current lang would introduce a lot of timing issues. (Though, since I'm not even sure a Document has such an attribute I can't tell if it can change dynamically). But anyway this doesn't seem to match what's being defined afterward.
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 hard to write a concise description of what inherit means here, so I am going for something that clearly conveys the intent without unnecessary details. Is your concern about clearly saying when the value will be looked up? I can see race conditions if the Document and worker are in different threads, but I can't see documents changing their language very often. (Maybe they do when the user agent translates the page - I will have to look at that.) Although I am also probably being too lax about accurate developer documentation. I'll take a stab at re-wording it for the next round.
incorrect (including using property-independent style sheet syntax like 'inherit' or 'initial'), | ||
then it must be ignored, without assigning a new font value. <ref>CSS</ref></p> | ||
must be assigned to the context, with the primary language set to the <span | ||
data-x="dom-context-2d-lang-used-value">used-value</span> for <code |
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.
Does this work to define the used value here? What if I don't ever set the font
attribute, but set the lang
one. Will my default sans-serif font be able to get this used value?
Would it work better if it was called in text preparation algo instead?
+Nit: might be better as "used value" (without the "-") for easing search in page
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 will take a look at what happens in Chromium for a non-defined font, but regardless I will update this to make it clear what happens when there is no defined font. And yes to moving it to the text preparation algorithm (and the direction resolution) because it does indeed need to be dynamic. I was wondering about the right way to ensure that dynamic changes were handled, and you've answered my question.
and with system fonts being computed to explicit values. If the new value is syntactically | ||
incorrect (including using property-independent style sheet syntax like 'inherit' or 'initial'), | ||
then it must be ignored, without assigning a new font value. <ref>CSS</ref></p> | ||
must be assigned to the context, with the primary language set to the <span |
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.
Is "primary language" here the same as CSS's content language?
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 was intending it to mean the "primary language" as used in https://html.spec.whatwg.org/multipage/dom.html#the-lang-and-xml:lang-attributes. The solution is to convert that linked reference to a definition and then refer to the definition in the canvas change.
attribute for="CanvasTextDrawingStyles"><code data-x="dom-context-2d-lang">lang</code></dfn> | ||
IDL attribute. On setting, <code data-x="dom-context-2d-lang">lang</code> must be | ||
a <span data-x="attr-lang">valid BCP 47 language tag</span> or the string "inherit". | ||
To determine the <dfn data-x="dom-context-2d-lang-used-value">used value</dfn> |
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.
Nit: It's a bit confusing to have this algo so close to the setter step. (Actually inside of it since there is no rendered return). It makes it look like it's called at that time only and thus doesn't make "inherit"
dynamic.
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.
Yes. I'll move it.
</ol> | ||
</li> | ||
|
||
<li><p>Otherwise, treat input to the <span>text preparation algorithm</span> as left-to-right text.</p></li> |
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.
With this being clarified here, the step 4. in the text preparation algo should be updated to use it directly (currently this algo wouldn't be called, if I'm reading correctly).
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.
Yes. As above.
Thanks for the very quick first round of review. I'll make the changes and update the PR. |
@@ -66410,15 +66426,56 @@ worker.postMessage(offscreenCanvas, [offscreenCanvas]);</code></pre> | |||
Notice that the font is only loaded inside the worker, and not in the document context.</p> | |||
</div> | |||
|
|||
<p>The choice of fonts and glyphs within a font may vary according to the <dfn | |||
attribute for="CanvasTextDrawingStyles"><code data-x="dom-context-2d-lang">lang</code></dfn> | |||
IDL attribute. On setting, <code data-x="dom-context-2d-lang">lang</code> must be |
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.
Should it be mentioned explicitly that if the string used for setting is invalid, then the stored value in the context doesn't change? For most other CanvasTextDrawingStyles
it isn't needed as they are defined using IDL enums, but for the attributes that need parsing, like letterSpacing
, the setter steps are outlined and therefore it's clear that an invalid value results in the attribute not being changed.
There is no way to localize the fonts used in HTML Canvas text rendering and metrics when using an
OffscreenCanvas
, and there is no information in the spec about how to determine the primary language even for regular 2D canvas (though implementations seem to agree to use the canvas element'slang
attribute and use the default locale from the HTTP header for Offscreen). This proposal adds alang
IDL attribute toCanvasTextDrawingStyles
, which is mixed into bothCanvasRenderingContext2D
andOffscreenCanvasRenderingContext2D
.The changes in this PR define how the language for font resolution should be chosen for canvas contexts and applies the same treatment to the
inherit
value for thedirection
IDL attribute which is currently not well defined.See the discussion in issue #10862.
It is an initial PR, subject to editing and the process for adding features.
/acknowledgements.html ( diff )
/canvas.html ( diff )