RESOLVED FIXED 168290
Web Inspector: RTL: number scripts are used inconsistently throughout the UI
https://bugs.webkit.org/show_bug.cgi?id=168290
Summary Web Inspector: RTL: number scripts are used inconsistently throughout the UI
Blaze Burg
Reported 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.
Attachments
SCREENSHOT (RTL) (14.33 KB, image/png)
2017-02-13 23:15 PST, Blaze Burg
no flags
Patch (1.27 KB, patch)
2017-03-16 13:10 PDT, Devin Rousso
no flags
Patch (10.59 KB, patch)
2017-03-17 00:15 PDT, Blaze Burg
joepeck: review+
Radar WebKit Bug Importer
Comment 1 2017-02-13 23:17:13 PST
Nikita Vasilyev
Comment 2 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
Blaze Burg
Comment 3 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.
Blaze Burg
Comment 4 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.
Devin Rousso
Comment 5 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.
Blaze Burg
Comment 6 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.
Blaze Burg
Comment 7 2017-03-17 00:15:08 PDT
Joseph Pecoraro
Comment 8 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?
Blaze Burg
Comment 9 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.
Blaze Burg
Comment 10 2017-03-25 16:08:43 PDT
Note You need to log in before you can comment on or make changes to this bug.