Bug 153962 - Web Inspector: UIString should take an optional key and description to aid localization
Summary: Web Inspector: UIString should take an optional key and description to aid lo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-02-07 09:36 PST by BJ Burg
Modified: 2018-12-20 10:29 PST (History)
6 users (show)

See Also:


Attachments
Patch (5.03 KB, patch)
2018-12-19 17:16 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (7.48 KB, patch)
2018-12-19 22:27 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (7.25 KB, patch)
2018-12-20 09:50 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2016-02-07 09:36:58 PST
Will probably need to update the JS string extraction script.
Comment 1 Radar WebKit Bug Importer 2016-02-07 09:37:10 PST
<rdar://problem/24542505>
Comment 2 Devin Rousso 2018-12-19 17:16:12 PST
Created attachment 357764 [details]
Patch
Comment 3 BJ Burg 2018-12-19 21:04:03 PST
Comment on attachment 357764 [details]
Patch

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

It would be nice if you could add a single usage of each form that we expect to work into the codebase, so that the script can be shown to work correctly.

> Tools/Scripts/extract-localizable-js-strings:-78
> -        print "$file:$.:WARNING: $&\n" while s/WI\.UIString\(.*?\)//;

Why did this warning get removed? It seems useful in the case where the arguments to UIString could not be parsed.

> Tools/Scripts/extract-localizable-js-strings:154
> +    $localizedStrings .= " // $commentByKey{$key}" if length $commentByKey{$key};

To make this consistent with Cocoa lproj files, please make the following changes:
- the comment should precede the localized string
- use /* multiline comment style */
Comment 4 Devin Rousso 2018-12-19 22:27:09 PST
Created attachment 357786 [details]
Patch
Comment 5 BJ Burg 2018-12-19 23:57:09 PST
Comment on attachment 357786 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/AuditTestGroupContentView.js:68
> +        const format = WI.UIString("%s%%", "audit-percentage-pass", "The number of tests that passed expressed as a percentage, followed by a literal %.");

Let's follow existing practice in Safari and use human-readable keys. In this case, something like "Percentage (of audits)". Ideally we wouldn't have a percent UIString floating by itself since it may interact with surrounding text, but I don't think it's a real problem in this specific instance.
Comment 6 Devin Rousso 2018-12-20 09:50:48 PST
Created attachment 357825 [details]
Patch
Comment 7 WebKit Commit Bot 2018-12-20 10:29:54 PST
Comment on attachment 357825 [details]
Patch

Clearing flags on attachment: 357825

Committed r239452: <https://trac.webkit.org/changeset/239452>
Comment 8 WebKit Commit Bot 2018-12-20 10:29:56 PST
All reviewed patches have been landed.  Closing bug.