Bug 153962

Summary: Web Inspector: UIString should take an optional key and description to aid localization
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: Devin Rousso <drousso>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, drousso, graouts, inspector-bugzilla-changes, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

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.