WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
168465
Web Inspector: Loc: Zoom level in Settings tab needs to use localized formatting
https://bugs.webkit.org/show_bug.cgi?id=168465
Summary
Web Inspector: Loc: Zoom level in Settings tab needs to use localized formatting
Blaze Burg
Reported
2017-02-16 14:16:35 PST
Intl to the rescue!
Attachments
[AFTER] tr-TR locale
(125.46 KB, image/png)
2017-02-16 14:17 PST
,
Blaze Burg
no flags
Details
Patch
(2.00 KB, patch)
2017-02-16 14:22 PST
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Patch v2
(1.75 KB, patch)
2017-02-16 15:10 PST
,
Blaze Burg
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Blaze Burg
Comment 1
2017-02-16 14:17:32 PST
STEPS TO REPRODUCE: - Open Settings Tab - Look at Zoom Level: section EXPECTED: With en-US locale, should be 100%, 120%, etc. With tr-TR locale, should be %100, %120, etc.
Blaze Burg
Comment 2
2017-02-16 14:17:51 PST
Created
attachment 301823
[details]
[AFTER] tr-TR locale
Blaze Burg
Comment 3
2017-02-16 14:22:47 PST
Created
attachment 301824
[details]
Patch
Joseph Pecoraro
Comment 4
2017-02-16 14:30:06 PST
Comment on
attachment 301824
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=301824&action=review
> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:188 > - option.textContent = `${Math.round(level * 100)}%`; > + option.textContent = formatter.format(level);
We have Number.percentageString in Utilities.js. Should we just use that?
Blaze Burg
Comment 5
2017-02-16 15:08:28 PST
(In reply to
comment #4
)
> Comment on
attachment 301824
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=301824&action=review
> > > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:188 > > - option.textContent = `${Math.round(level * 100)}%`; > > + option.textContent = formatter.format(level); > > We have Number.percentageString in Utilities.js. Should we just use that?
Had no idea. Yes, it works the same it seems.
Blaze Burg
Comment 6
2017-02-16 15:10:43 PST
Created
attachment 301836
[details]
Patch v2
Matt Baker
Comment 7
2017-02-17 09:55:02 PST
Comment on
attachment 301836
[details]
Patch v2 Text isn't being localized with the patch applied
Blaze Burg
Comment 8
2017-02-17 14:00:57 PST
(In reply to
comment #1
)
> STEPS TO REPRODUCE: > - Open Settings Tab > - Look at Zoom Level: section > > EXPECTED: > > With en-US locale, should be 100%, 120%, etc. > With tr-TR locale, should be %100, %120, etc.
To get tr-TR locale on macOS, go to System Preferences > Language & Region and add Turkish/Türkçe, then drag it to the top position. Relaunch Safari.
Blaze Burg
Comment 9
2017-02-17 14:01:13 PST
(In reply to
comment #7
)
> Comment on
attachment 301836
[details]
> Patch v2 > > Text isn't being localized with the patch applied
It works for me. Did you change the system language?
Matt Baker
Comment 10
2017-02-17 14:15:47 PST
(In reply to
comment #9
)
> (In reply to
comment #7
) > > Comment on
attachment 301836
[details]
> > Patch v2 > > > > Text isn't being localized with the patch applied > > It works for me. Did you change the system language?
Works! My mistake.
WebKit Commit Bot
Comment 11
2017-02-17 14:41:27 PST
Comment on
attachment 301836
[details]
Patch v2 Clearing flags on attachment: 301836 Committed
r212578
: <
http://trac.webkit.org/changeset/212578
>
WebKit Commit Bot
Comment 12
2017-02-17 14:41:32 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug