Bug 168290 - Web Inspector: RTL: number scripts are used inconsistently throughout the UI
Summary: Web Inspector: RTL: number scripts are used inconsistently throughout the UI
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: Other
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-02-13 23:15 PST by BJ Burg
Modified: 2017-03-25 16:08 PDT (History)
11 users (show)

See Also:


Attachments
SCREENSHOT (RTL) (14.33 KB, image/png)
2017-02-13 23:15 PST, BJ Burg
no flags Details
Patch (1.27 KB, patch)
2017-03-16 13:10 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (10.59 KB, patch)
2017-03-17 00:15 PDT, BJ Burg
joepeck: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-02-13 23:15:59 PST
Created attachment 301468 [details]
SCREENSHOT (RTL)

I believe we need to put all user-visible numbers without special meaning (like HTTP 404) through the Intl API, which produces hindi numbers as expected when using Arabic. See dashboard screenshot.
Comment 1 Radar WebKit Bug Importer 2017-02-13 23:17:13 PST
<rdar://problem/30508263>
Comment 2 Nikita Vasilyev 2017-02-16 10:42:04 PST
Are the numerals on the screenshot really "hindi numbers"? I’m trying to educate myself and these look like Eastern Arabic to me.

https://en.wikipedia.org/wiki/Eastern_Arabic_numerals
https://en.wikibooks.org/wiki/Hindi/Numbers
Comment 3 BJ Burg 2017-02-16 11:51:38 PST
(In reply to comment #2)
> Are the numerals on the screenshot really "hindi numbers"? I’m trying to
> educate myself and these look like Eastern Arabic to me.
> 
> https://en.wikipedia.org/wiki/Eastern_Arabic_numerals
> https://en.wikibooks.org/wiki/Hindi/Numbers

Oops, you are right.
Comment 4 BJ Burg 2017-02-16 11:57:30 PST
On macOS, number fields with steppers also produce these numerals. So in the Settings tab we need to support that if WebKit supports it, and otherwise leave in western numerals.

As for the Visual Styles Sidebar, we should probably stick with western numerals for anything related to CSS because the underlying assumption is that the editors map directly to CSS property declarations.

There are also some other values not localized properly like Zoom 100%, which should appear as %100 in Turkish locales. We can fix that in a separate bug if needed.
Comment 5 Devin Rousso 2017-03-16 13:10:51 PDT
Created attachment 304681 [details]
Patch

Is there a good way to test this?  I tried setting my system language to Arabic and it didn't really change anything.
Comment 6 BJ Burg 2017-03-16 15:24:37 PDT
(In reply to comment #5)
> Created attachment 304681 [details]
> Patch
> 
> Is there a good way to test this?  I tried setting my system language to
> Arabic and it didn't really change anything.

You need to do this, then relaunch Safari. Engineering builds do not use UIString localizations, but the JS Intl APIs will pick up the new LC_LOCALE and format number differently. It's easiest to see some numerals by making a timeline recording.
Comment 7 BJ Burg 2017-03-17 00:15:08 PDT
Created attachment 304760 [details]
Patch
Comment 8 Joseph Pecoraro 2017-03-24 11:02:18 PDT
Comment on attachment 304760 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/DataGridNode.js:387
> +            return "\u200b"; // Zero width space to keep the cell from collapsing.

Maybe we should make a named variable for this in Utilities.js (like emDash).

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:754
> -            this._timelineRuler.formatLabelCallback = (value) => value.toFixed(0);
> +            this._timelineRuler.formatLabelCallback = (value) => value.maxDecimals(0).toLocaleString();

I understood all the other changes, but why the toFixed -> maxDecimals here?
Comment 9 BJ Burg 2017-03-24 12:09:03 PDT
Comment on attachment 304760 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/DataGridNode.js:387
>> +            return "\u200b"; // Zero width space to keep the cell from collapsing.
> 
> Maybe we should make a named variable for this in Utilities.js (like emDash).

OK. There are some bidi override characters we might need to use too.

>> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:754
>> +            this._timelineRuler.formatLabelCallback = (value) => value.maxDecimals(0).toLocaleString();
> 
> I understood all the other changes, but why the toFixed -> maxDecimals here?

I found it clearer to understand.
Comment 10 BJ Burg 2017-03-25 16:08:43 PDT
Committed r214405: <http://trac.webkit.org/changeset/214405>