Bug 220474 – Web Inspector: Font Details sidebar - Fractional variation axis ranges/default values are rounded.

• Component: Web Inspector
• Status: RESOLVED FIXED

[Patrick Angle]

Created attachment 417291 [details]
Screenshot before patch

Inspect the font sample on: https://v-fonts.com/fonts/skia. Skia’s `wght` and `wdth` registered axes have ranges of 0.47–3.19 and 0.619–1.3, respectively. They are instead listed as 0-3 and 0-1.

[Radar WebKit Bug Importer]

<rdar://problem/72940358>

[Patrick Angle]

Created attachment 417654 [details]
Patch v1.0

[Patrick Angle]

Created attachment 417656 [details]
Screenshot of Patch v1.0

[Devin Rousso]

Comment on attachment 417654 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417654&action=review

r=me

> Source/WebInspectorUI/ChangeLog:14
> +        - Add basic support for IEEE printf `g` which truncates trailing zeros in the fractional portion of the number.

Are we sure that localizers will know what this is?

Aside: I wonder if we actually want to use this instead of `f` more often 🤔

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:1096
> +        g: function(substitution, token)

Please add some tests for this :)

IIRC there's tests for other stuff in `LayoutTests/inspector/unit-tests/string-utilities.html`.

Style: you should be able to `g(substitution, token)` instead of having the `: function` :)

[Patrick Angle]

Comment on attachment 417654 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417654&action=review

>> Source/WebInspectorUI/ChangeLog:14
>> +        - Add basic support for IEEE printf `g` which truncates trailing zeros in the fractional portion of the number.
> 
> Are we sure that localizers will know what this is?
> 
> Aside: I wonder if we actually want to use this instead of `f` more often 🤔

I'm not sure. Should I just note this in the string descriptions? I'm not sure the best way to communicate this otherwise.

[Devin Rousso]

Comment on attachment 417654 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417654&action=review

>>> Source/WebInspectorUI/ChangeLog:14
>>> +        - Add basic support for IEEE printf `g` which truncates trailing zeros in the fractional portion of the number.
>> 
>> Are we sure that localizers will know what this is?
>> 
>> Aside: I wonder if we actually want to use this instead of `f` more often 🤔
> 
> I'm not sure. Should I just note this in the string descriptions? I'm not sure the best way to communicate this otherwise.

I'd look at some of our older "fix a localization bug" changes and maybe email some of the people involved?  Perhaps Greg or someone working on Safari knows who to contact.

[Patrick Angle]

Comment on attachment 417654 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417654&action=review

>>>> Source/WebInspectorUI/ChangeLog:14
>>>> +        - Add basic support for IEEE printf `g` which truncates trailing zeros in the fractional portion of the number.
>>> 
>>> Are we sure that localizers will know what this is?
>>> 
>>> Aside: I wonder if we actually want to use this instead of `f` more often 🤔
>> 
>> I'm not sure. Should I just note this in the string descriptions? I'm not sure the best way to communicate this otherwise.
> 
> I'd look at some of our older "fix a localization bug" changes and maybe email some of the people involved?  Perhaps Greg or someone working on Safari knows who to contact.

In looking this up in our own docs (https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html) which in turn references (http://www.opengroup.org/onlinepubs/009695399/functions/printf.html), I'm surprised to find that the behavior I've always taken for granted with `g` is actually a side effect, and the main purpose is actually to selectively use exponential form. There doesn't appear to be a clear analogue to the behavior I'm trying to achieve in `printf` parlance. 😔

This leads me down a path where maybe it makes more sense to have a utility function on Number (e.g. `Number.decimalString(fraction, maximumPrecision, minimumPrecision)` or similar?) for this functionality. This patch is one of those cases where for 90%+ of fonts I don't expect any fractions, and the clutter makes it more difficult to read the values at a glance. Thoughts?

[Devin Rousso]

Comment on attachment 417654 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417654&action=review

>>>>> Source/WebInspectorUI/ChangeLog:14
>>>>> +        - Add basic support for IEEE printf `g` which truncates trailing zeros in the fractional portion of the number.
>>>> 
>>>> Are we sure that localizers will know what this is?
>>>> 
>>>> Aside: I wonder if we actually want to use this instead of `f` more often 🤔
>>> 
>>> I'm not sure. Should I just note this in the string descriptions? I'm not sure the best way to communicate this otherwise.
>> 
>> I'd look at some of our older "fix a localization bug" changes and maybe email some of the people involved?  Perhaps Greg or someone working on Safari knows who to contact.
> 
> In looking this up in our own docs (https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html) which in turn references (http://www.opengroup.org/onlinepubs/009695399/functions/printf.html), I'm surprised to find that the behavior I've always taken for granted with `g` is actually a side effect, and the main purpose is actually to selectively use exponential form. There doesn't appear to be a clear analogue to the behavior I'm trying to achieve in `printf` parlance. 😔
> 
> This leads me down a path where maybe it makes more sense to have a utility function on Number (e.g. `Number.decimalString(fraction, maximumPrecision, minimumPrecision)` or similar?) for this functionality. This patch is one of those cases where for 90%+ of fonts I don't expect any fractions, and the clutter makes it more difficult to read the values at a glance. Thoughts?

Maybe just use `Number.maxDecimals` (another one of our utilities) and `%s` instead?

[Patrick Angle]

Comment on attachment 417654 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417654&action=review

>>>>>> Source/WebInspectorUI/ChangeLog:14
>>>>>> +        - Add basic support for IEEE printf `g` which truncates trailing zeros in the fractional portion of the number.
>>>>> 
>>>>> Are we sure that localizers will know what this is?
>>>>> 
>>>>> Aside: I wonder if we actually want to use this instead of `f` more often 🤔
>>>> 
>>>> I'm not sure. Should I just note this in the string descriptions? I'm not sure the best way to communicate this otherwise.
>>> 
>>> I'd look at some of our older "fix a localization bug" changes and maybe email some of the people involved?  Perhaps Greg or someone working on Safari knows who to contact.
>> 
>> In looking this up in our own docs (https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html) which in turn references (http://www.opengroup.org/onlinepubs/009695399/functions/printf.html), I'm surprised to find that the behavior I've always taken for granted with `g` is actually a side effect, and the main purpose is actually to selectively use exponential form. There doesn't appear to be a clear analogue to the behavior I'm trying to achieve in `printf` parlance. 😔
>> 
>> This leads me down a path where maybe it makes more sense to have a utility function on Number (e.g. `Number.decimalString(fraction, maximumPrecision, minimumPrecision)` or similar?) for this functionality. This patch is one of those cases where for 90%+ of fonts I don't expect any fractions, and the clutter makes it more difficult to read the values at a glance. Thoughts?
> 
> Maybe just use `Number.maxDecimals` (another one of our utilities) and `%s` instead?

I don't think that'll work here, since the decimal point needs to be localized still, and `maxDecimals` just rounds the number off. We would need to pass through `toLocaleString` at some point, with a `minimumFractionDigits` of `0` and a `maximumFractionDigits` of `2`, at which point the rounding wasn't necessary.

[Devin Rousso]

Comment on attachment 417654 [details]
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=417654&action=review

>>>>>>> Source/WebInspectorUI/ChangeLog:14
>>>>>>> +        - Add basic support for IEEE printf `g` which truncates trailing zeros in the fractional portion of the number.
>>>>>> 
>>>>>> Are we sure that localizers will know what this is?
>>>>>> 
>>>>>> Aside: I wonder if we actually want to use this instead of `f` more often 🤔
>>>>> 
>>>>> I'm not sure. Should I just note this in the string descriptions? I'm not sure the best way to communicate this otherwise.
>>>> 
>>>> I'd look at some of our older "fix a localization bug" changes and maybe email some of the people involved?  Perhaps Greg or someone working on Safari knows who to contact.
>>> 
>>> In looking this up in our own docs (https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html) which in turn references (http://www.opengroup.org/onlinepubs/009695399/functions/printf.html), I'm surprised to find that the behavior I've always taken for granted with `g` is actually a side effect, and the main purpose is actually to selectively use exponential form. There doesn't appear to be a clear analogue to the behavior I'm trying to achieve in `printf` parlance. 😔
>>> 
>>> This leads me down a path where maybe it makes more sense to have a utility function on Number (e.g. `Number.decimalString(fraction, maximumPrecision, minimumPrecision)` or similar?) for this functionality. This patch is one of those cases where for 90%+ of fonts I don't expect any fractions, and the clutter makes it more difficult to read the values at a glance. Thoughts?
>> 
>> Maybe just use `Number.maxDecimals` (another one of our utilities) and `%s` instead?
> 
> I don't think that'll work here, since the decimal point needs to be localized still, and `maxDecimals` just rounds the number off. We would need to pass through `toLocaleString` at some point, with a `minimumFractionDigits` of `0` and a `maximumFractionDigits` of `2`, at which point the rounding wasn't necessary.

Well if need be we could just have this be a special case and call `.toLocaleString(undefined, {minimumFractionDigits: 0, maximumFractionDigits: 2})` where needed in `Source/WebInspectorUI/UserInterface/Views/FontDetailsPanel.js` (maybe even make a private `_formatVariationValue`).

[Patrick Angle]

Created attachment 417701 [details]
Patch v1.5 - Revised approach for converting fractions to strings

[Blaze Burg]

Comment on attachment 417701 [details]
Patch v1.5 - Revised approach for converting fractions to strings

View in context: https://bugs.webkit.org/attachment.cgi?id=417701&action=review

r=me

> Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js:5
> +localizedStrings[" (Range: %s-%s, Default: %s) @ Font Details Sidebar"] = " (Range: %s-%s, Default: %s)";

Hopefully localizers can figure out the context based on the string.

> Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js:78
> +localizedStrings["%s%%"] = "%s%%";

It would be wise to explain what this is with a localization comment.

[Patrick Angle]

Created attachment 417910 [details]
Patch v1.6 - Review Notes

[EWS]

Committed r271620: <https://trac.webkit.org/changeset/271620>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417910 [details].