Bug 110095 – Web Inspector: Specify return type of WebInspector.UIString

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Eugene Klyuchnikov]

1) Extract "fragment builder" from AuditsPanel.
This method builds DOM fragment by format and substitutions (and optional formatters).
In case of standard string formatters we can use fact that "%s" means literal substitution.

2) Extract "localize" from WebInspector.UIString. Fragment builder should be supplied with localized
formats. This was impossible because all localization logic was baked in WebInspector.UIString.

[Eugene Klyuchnikov]

Created attachment 188825 [details]
Patch

[Pavel Feldman]

Comment on attachment 188825 [details]
Patch

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

> Source/WebCore/inspector/front-end/UIString.js:36
> +WebInspector.localize = function(string) {

Should be private.

> Source/WebCore/inspector/front-end/UIUtils.js:1106
> +WebInspector.formatFragment = function(format, substitutions, formatters) {

{ on the next line.

If that is extracted, where is the updated client?

[Eugene Klyuchnikov]

With extracted _localize method typed code ration increases on 0.4%

[Eugene Klyuchnikov]

Comment on attachment 188825 [details]
Patch

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

>> Source/WebCore/inspector/front-end/UIString.js:36
>> +WebInspector.localize = function(string) {
> 
> Should be private.

Done

>> Source/WebCore/inspector/front-end/UIUtils.js:1106
>> +WebInspector.formatFragment = function(format, substitutions, formatters) {
> 
> { on the next line.
> 
> If that is extracted, where is the updated client?

Take a look at AuditRuleResult.addFormatted

[Eugene Klyuchnikov]

Created attachment 188846 [details]
Patch

[Pavel Feldman]

Comment on attachment 188846 [details]
Patch

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

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:554
> +                        console.error(WebInspector.UIString("Unknown scope type: \"%s\"", scope.type));

Why is this a part of the change?

> Source/WebCore/inspector/front-end/UIString.js:59
> +    return String.vsprintf(WebInspector._localize(string), Array.prototype.slice.call(arguments, 1));

Why do you extract it?

[Eugene Klyuchnikov]

Comment on attachment 188846 [details]
Patch

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

>> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:554
>> +                        console.error(WebInspector.UIString("Unknown scope type: \"%s\"", scope.type));
> 
> Why is this a part of the change?

Otherwise js compiler will report type mismatch warning

>> Source/WebCore/inspector/front-end/UIString.js:59
>> +    return String.vsprintf(WebInspector._localize(string), Array.prototype.slice.call(arguments, 1));
> 
> Why do you extract it?

Localize could be used in cases when we need to use power of String.format but still be localized.

[Eugene Klyuchnikov]

Specify return type of WebInspector.UIString.
Fix new js-compiler warnings.

This change will increase type coverage by 0.4% (to 81%)

[Eugene Klyuchnikov]

Created attachment 189504 [details]
Patch

[Pavel Feldman]

Comment on attachment 189504 [details]
Patch

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

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:554
> +                        console.error(WebInspector.UIString("Unknown scope type: \"%s\"", scope.type));

There is no need to localize it. Otherwise looks good.

[Eugene Klyuchnikov]

Created attachment 190286 [details]
Patch

[Eugene Klyuchnikov]

Comment on attachment 189504 [details]
Patch

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

>> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:554
>> +                        console.error(WebInspector.UIString("Unknown scope type: \"%s\"", scope.type));
> 
> There is no need to localize it. Otherwise looks good.

Fixed.

[Eugene Klyuchnikov]

Committed r144061: <http://trac.webkit.org/changeset/144061>