Commit 7a329cb7 authored by Lorne Mitchell's avatar Lorne Mitchell Committed by Commit Bot

DevTools: Enabled check_localization presubmit script

* Enabled the check_localization presubmit script.
* Fixed localization issues caught by the check_localization presubmit script.
* Updated the check_localization script to allow contatenation of non-alphabetic strings with localized strings.
  * For example, ls`Status Code` + ": " is a valid concatenation. This allows for decorations to be concatenated with localized strings.

Change-Id: I741940c9ebdac363ac0ccad3f7de20d508204e2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1544878
Auto-Submit: Lorne Mitchell <lomitch@microsoft.com>
Reviewed-by: default avatarJoel Einbinder <einbinder@chromium.org>
Reviewed-by: default avatarPavel Feldman <pfeldman@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#647134}
parent a3bc334b
...@@ -202,7 +202,7 @@ def CheckChangeOnUpload(input_api, output_api): ...@@ -202,7 +202,7 @@ def CheckChangeOnUpload(input_api, output_api):
results = [] results = []
results.extend(_CheckBuildGN(input_api, output_api)) results.extend(_CheckBuildGN(input_api, output_api))
results.extend(_CheckFormat(input_api, output_api)) results.extend(_CheckFormat(input_api, output_api))
# results.extend(_CheckDevtoolsLocalization(input_api, output_api)) results.extend(_CheckDevtoolsLocalization(input_api, output_api))
results.extend(_CheckDevtoolsStyle(input_api, output_api)) results.extend(_CheckDevtoolsStyle(input_api, output_api))
results.extend(_CompileDevtoolsFrontend(input_api, output_api)) results.extend(_CompileDevtoolsFrontend(input_api, output_api))
results.extend(_CheckConvertSVGToPNGHashes(input_api, output_api)) results.extend(_CheckConvertSVGToPNGHashes(input_api, output_api))
......
...@@ -97,8 +97,7 @@ Audits2.AuditController = class extends Common.Object { ...@@ -97,8 +97,7 @@ Audits2.AuditController = class extends Common.Object {
const inspectedURL = mainTarget && mainTarget.inspectedURL(); const inspectedURL = mainTarget && mainTarget.inspectedURL();
if (inspectedURL && !/^(http|chrome-extension)/.test(inspectedURL)) { if (inspectedURL && !/^(http|chrome-extension)/.test(inspectedURL)) {
return Common.UIString( return Common.UIString(
'Can only audit HTTP/HTTPS pages and Chrome extensions. ' + 'Can only audit HTTP/HTTPS pages and Chrome extensions. Navigate to a different page to start an audit.');
'Navigate to a different page to start an audit.');
} }
return null; return null;
...@@ -179,8 +178,7 @@ Audits2.AuditController = class extends Common.Object { ...@@ -179,8 +178,7 @@ Audits2.AuditController = class extends Common.Object {
let helpText = ''; let helpText = '';
if (hasActiveServiceWorker) { if (hasActiveServiceWorker) {
helpText = Common.UIString( helpText = Common.UIString(
'Multiple tabs are being controlled by the same service worker. ' + 'Multiple tabs are being controlled by the same service worker. Close your other tabs on the same origin to audit this page.');
'Close your other tabs on the same origin to audit this page.');
} else if (!hasAtLeastOneCategory) { } else if (!hasAtLeastOneCategory) {
helpText = Common.UIString('At least one category must be selected.'); helpText = Common.UIString('At least one category must be selected.');
} else if (unauditablePageMessage) { } else if (unauditablePageMessage) {
......
...@@ -223,8 +223,7 @@ Audits2.StatusView = class { ...@@ -223,8 +223,7 @@ Audits2.StatusView = class {
this._statusText.createChild('p').createTextChild(Common.UIString('Ah, sorry! We ran into an error.')); this._statusText.createChild('p').createTextChild(Common.UIString('Ah, sorry! We ran into an error.'));
if (Audits2.StatusView.KnownBugPatterns.some(pattern => pattern.test(err.message))) { if (Audits2.StatusView.KnownBugPatterns.some(pattern => pattern.test(err.message))) {
const message = Common.UIString( const message = Common.UIString(
'Try to navigate to the URL in a fresh Chrome profile without any other tabs or ' + 'Try to navigate to the URL in a fresh Chrome profile without any other tabs or extensions open and try again.');
'extensions open and try again.');
this._statusText.createChild('p').createTextChild(message); this._statusText.createChild('p').createTextChild(message);
} else { } else {
this._renderBugReportBody(err, this._inspectedURL); this._renderBugReportBody(err, this._inspectedURL);
......
...@@ -76,8 +76,7 @@ Coverage.CoverageView = class extends UI.VBox { ...@@ -76,8 +76,7 @@ Coverage.CoverageView = class extends UI.VBox {
if (this._startWithReloadButton) { if (this._startWithReloadButton) {
const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButtonForId('coverage.start-with-reload')); const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButtonForId('coverage.start-with-reload'));
message = UI.formatLocalized( message = UI.formatLocalized(
'Click the record button %s to start capturing coverage.\n' + 'Click the record button %s to start capturing coverage.\nClick the reload button %s to reload and start capturing coverage.',
'Click the reload button %s to reload and start capturing coverage.',
[recordButton, reloadButton]); [recordButton, reloadButton]);
} else { } else {
message = UI.formatLocalized('Click the record button %s to start capturing coverage.', [recordButton]); message = UI.formatLocalized('Click the record button %s to start capturing coverage.', [recordButton]);
......
...@@ -115,9 +115,10 @@ Network.ResourceWebSocketFrameView = class extends UI.VBox { ...@@ -115,9 +115,10 @@ Network.ResourceWebSocketFrameView = class extends UI.VBox {
* @return {string} * @return {string}
*/ */
static opCodeDescription(opCode, mask) { static opCodeDescription(opCode, mask) {
const rawDescription = Network.ResourceWebSocketFrameView.opCodeDescriptions[opCode] || ''; const localizedDescription = Network.ResourceWebSocketFrameView.opCodeDescriptions[opCode] || '';
const localizedDescription = Common.UIString(rawDescription); if (mask)
return Common.UIString('%s (Opcode %d%s)', localizedDescription, opCode, (mask ? ', mask' : '')); return ls`${localizedDescription} (Opcode ${opCode}, mask)`;
return ls`${localizedDescription} (Opcode ${opCode})`;
} }
/** /**
...@@ -230,12 +231,12 @@ Network.ResourceWebSocketFrameView.OpCodes = { ...@@ -230,12 +231,12 @@ Network.ResourceWebSocketFrameView.OpCodes = {
Network.ResourceWebSocketFrameView.opCodeDescriptions = (function() { Network.ResourceWebSocketFrameView.opCodeDescriptions = (function() {
const opCodes = Network.ResourceWebSocketFrameView.OpCodes; const opCodes = Network.ResourceWebSocketFrameView.OpCodes;
const map = []; const map = [];
map[opCodes.ContinuationFrame] = 'Continuation Frame'; map[opCodes.ContinuationFrame] = ls`Continuation Frame`;
map[opCodes.TextFrame] = 'Text Message'; map[opCodes.TextFrame] = ls`Text Message`;
map[opCodes.BinaryFrame] = 'Binary Message'; map[opCodes.BinaryFrame] = ls`Binary Message`;
map[opCodes.ContinuationFrame] = 'Connection Close Message'; map[opCodes.ContinuationFrame] = ls`Connection Close Message`;
map[opCodes.PingFrame] = 'Ping Message'; map[opCodes.PingFrame] = ls`Ping Message`;
map[opCodes.PongFrame] = 'Pong Message'; map[opCodes.PongFrame] = ls`Pong Message`;
return map; return map;
})(); })();
...@@ -272,7 +273,7 @@ Network.ResourceWebSocketFrameNode = class extends DataGrid.SortableDataGridNode ...@@ -272,7 +273,7 @@ Network.ResourceWebSocketFrameNode = class extends DataGrid.SortableDataGridNode
} else if (frame.opCode === Network.ResourceWebSocketFrameView.OpCodes.BinaryFrame) { } else if (frame.opCode === Network.ResourceWebSocketFrameView.OpCodes.BinaryFrame) {
length = Number.bytesToString(base64ToSize(frame.text)); length = Number.bytesToString(base64ToSize(frame.text));
description = 'Binary Message'; description = Network.ResourceWebSocketFrameView.opCodeDescriptions[frame.opCode];
} else { } else {
dataText = description; dataText = description;
......
...@@ -858,8 +858,7 @@ SDK.NetworkDispatcher = class { ...@@ -858,8 +858,7 @@ SDK.NetworkDispatcher = class {
if (shouldReportCorbBlocking) { if (shouldReportCorbBlocking) {
const message = Common.UIString( const message = Common.UIString(
`Cross-Origin Read Blocking (CORB) blocked cross-origin response %s with MIME type %s. ` + `Cross-Origin Read Blocking (CORB) blocked cross-origin response %s with MIME type %s. See https://www.chromestatus.com/feature/5629709824032768 for more details.`,
`See https://www.chromestatus.com/feature/5629709824032768 for more details.`,
networkRequest.url(), networkRequest.mimeType); networkRequest.url(), networkRequest.mimeType);
this._manager.dispatchEventToListeners( this._manager.dispatchEventToListeners(
SDK.NetworkManager.Events.MessageGenerated, SDK.NetworkManager.Events.MessageGenerated,
...@@ -868,10 +867,18 @@ SDK.NetworkDispatcher = class { ...@@ -868,10 +867,18 @@ SDK.NetworkDispatcher = class {
if (Common.moduleSetting('monitoringXHREnabled').get() && if (Common.moduleSetting('monitoringXHREnabled').get() &&
networkRequest.resourceType().category() === Common.resourceCategories.XHR) { networkRequest.resourceType().category() === Common.resourceCategories.XHR) {
const message = Common.UIString( let message;
(networkRequest.failed || networkRequest.hasErrorStatusCode()) ? '%s failed loading: %s "%s".' : const failedToLoad = networkRequest.failed || networkRequest.hasErrorStatusCode();
'%s finished loading: %s "%s".', if (failedToLoad) {
networkRequest.resourceType().title(), networkRequest.requestMethod, networkRequest.url()); message = Common.UIString(
'%s failed loading: %s "%s".', networkRequest.resourceType().title(), networkRequest.requestMethod,
networkRequest.url());
} else {
message = Common.UIString(
'%s finished loading: %s "%s".', networkRequest.resourceType().title(), networkRequest.requestMethod,
networkRequest.url());
}
this._manager.dispatchEventToListeners( this._manager.dispatchEventToListeners(
SDK.NetworkManager.Events.MessageGenerated, SDK.NetworkManager.Events.MessageGenerated,
{message: message, requestId: networkRequest.requestId(), warning: false}); {message: message, requestId: networkRequest.requestId(), warning: false});
......
...@@ -29,11 +29,9 @@ Sources.GoToLineQuickOpen = class extends QuickOpen.FilteredListWidget.Provider ...@@ -29,11 +29,9 @@ Sources.GoToLineQuickOpen = class extends QuickOpen.FilteredListWidget.Provider
const position = this._parsePosition(query); const position = this._parsePosition(query);
if (!position) if (!position)
return Common.UIString('Type a number to go to that line.'); return Common.UIString('Type a number to go to that line.');
let text = Common.UIString('Go to line ') + position.line;
if (position.column && position.column > 1) if (position.column && position.column > 1)
text += Common.UIString(' and column ') + position.column; return ls`Go to line ${position.line} and column ${position.column}.`;
text += '.'; return ls`Go to line ${position.line}.`;
return text;
} }
/** /**
......
...@@ -460,8 +460,7 @@ Sources.SourcesPanel = class extends UI.Panel { ...@@ -460,8 +460,7 @@ Sources.SourcesPanel = class extends UI.Panel {
_pauseOnExceptionEnabledChanged() { _pauseOnExceptionEnabledChanged() {
const enabled = Common.moduleSetting('pauseOnExceptionEnabled').get(); const enabled = Common.moduleSetting('pauseOnExceptionEnabled').get();
this._pauseOnExceptionButton.setToggled(enabled); this._pauseOnExceptionButton.setToggled(enabled);
this._pauseOnExceptionButton.setTitle( this._pauseOnExceptionButton.setTitle(enabled ? ls`Don't pause on exceptions` : ls`Pause on exceptions`);
Common.UIString(enabled ? 'Don\'t pause on exceptions' : 'Pause on exceptions'));
this._debugToolbarDrawer.classList.toggle('expanded', enabled); this._debugToolbarDrawer.classList.toggle('expanded', enabled);
} }
......
...@@ -45,7 +45,7 @@ Sources.ThreadsSidebarPane = class extends UI.VBox { ...@@ -45,7 +45,7 @@ Sources.ThreadsSidebarPane = class extends UI.VBox {
} }
function updatePausedState() { function updatePausedState() {
pausedState.textContent = Common.UIString(debuggerModel.isPaused() ? 'paused' : ''); pausedState.textContent = debuggerModel.isPaused() ? ls`paused` : '';
} }
/** /**
......
...@@ -655,13 +655,11 @@ Timeline.TimelinePanel = class extends UI.Panel { ...@@ -655,13 +655,11 @@ Timeline.TimelinePanel = class extends UI.Panel {
const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButtonForId('timeline.record-reload')); const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButtonForId('timeline.record-reload'));
centered.createChild('p').appendChild(UI.formatLocalized( centered.createChild('p').appendChild(UI.formatLocalized(
'Click the record button %s or hit %s to start a new recording.\n' + 'Click the record button %s or hit %s to start a new recording.\nClick the reload button %s or hit %s to record the page load.',
'Click the reload button %s or hit %s to record the page load.',
[recordButton, recordKey, reloadButton, reloadKey])); [recordButton, recordKey, reloadButton, reloadKey]));
centered.createChild('p').appendChild(UI.formatLocalized( centered.createChild('p').appendChild(UI.formatLocalized(
'After recording, select an area of interest in the overview by dragging.\n' + 'After recording, select an area of interest in the overview by dragging.\nThen, zoom and pan the timeline with the mousewheel or %s keys.\n%s',
'Then, zoom and pan the timeline with the mousewheel or %s keys.\n%s',
[navigateNode, learnMoreNode])); [navigateNode, learnMoreNode]));
this._landingPage.show(this._statusPaneContainer); this._landingPage.show(this._statusPaneContainer);
......
...@@ -556,10 +556,13 @@ Timeline.TimelineUIUtils = class { ...@@ -556,10 +556,13 @@ Timeline.TimelineUIUtils = class {
break; break;
} }
case recordType.ParseHTML: { case recordType.ParseHTML: {
const startLine = event.args['beginData']['startLine'];
const endLine = event.args['endData'] && event.args['endData']['endLine']; const endLine = event.args['endData'] && event.args['endData']['endLine'];
const url = Bindings.displayNameForURL(event.args['beginData']['url']); const url = Bindings.displayNameForURL(event.args['beginData']['url']);
detailsText = Common.UIString( if (endLine >= 0)
'%s [%s\u2026%s]', url, event.args['beginData']['startLine'] + 1, endLine >= 0 ? endLine + 1 : ''); detailsText = Common.UIString('%s [%s\u2026%s]', url, startLine + 1, endLine + 1);
else
detailsText = Common.UIString('%s [%s\u2026]', url, startLine + 1);
break; break;
} }
case recordType.CompileModule: case recordType.CompileModule:
......
...@@ -153,19 +153,48 @@ function getLocation(node) { ...@@ -153,19 +153,48 @@ function getLocation(node) {
return ''; return '';
} }
function buildConcatenatedNodesList(node, nodes) {
if (!node)
return;
if (node.left === undefined && node.right === undefined) {
nodes.push(node);
return;
}
buildConcatenatedNodesList(node.left, nodes);
buildConcatenatedNodesList(node.right, nodes);
}
/** /**
* Recursively check if there is concatenation to localization call. * Recursively check if there is concatenation to localization call.
* Concatenation is allowed between localized strings and non-alphabetic strings.
* It is not allowed between a localized string and a word.
* Example (allowed): ls`Status Code` + ": "
* Example (disallowed): ls`Status` + " Code" + ": "
*/ */
function checkConcatenation(node, filePath, errors) { function checkConcatenation(parentNode, node, filePath, errors) {
if (node !== undefined && node.type === esprimaTypes.BI_EXPR && node.operator === '+') { function isWord(node) {
const code = escodegen.generate(node); return (node.type === 'Literal' && !!node.value.match(/[a-z]/i));
if (isLocalizationCall(node.left) || isLocalizationCall(node.right)) { }
addError( function isConcatenation(node) {
`${filePath}${getLocation(node)}: string concatenation should be changed to variable substitution with ls: ${ return (node !== undefined && node.type === esprimaTypes.BI_EXPR && node.operator === '+');
code}`, }
errors);
} else { if (isConcatenation(parentNode))
[node.left, node.right].forEach(node => checkConcatenation(node, filePath, errors)); return;
if (isConcatenation(node)) {
let concatenatedNodes = [];
buildConcatenatedNodesList(node, concatenatedNodes);
const hasLocalizationCall = !!concatenatedNodes.find(currentNode => isLocalizationCall(currentNode));
if (hasLocalizationCall) {
const hasAlphabeticLiteral = !!concatenatedNodes.find(currentNode => isWord(currentNode));
if (hasAlphabeticLiteral) {
const code = escodegen.generate(node);
addError(
`${filePath}${
getLocation(node)}: string concatenation should be changed to variable substitution with ls: ${code}`,
errors);
}
} }
} }
} }
...@@ -186,6 +215,10 @@ function checkFunctionArgument(functionName, argumentIndex, node, filePath, erro ...@@ -186,6 +215,10 @@ function checkFunctionArgument(functionName, argumentIndex, node, filePath, erro
if (node !== undefined && node.type === esprimaTypes.CALL_EXPR && verifyFunctionCallee(node.callee, functionName) && if (node !== undefined && node.type === esprimaTypes.CALL_EXPR && verifyFunctionCallee(node.callee, functionName) &&
node.arguments !== undefined && node.arguments.length > argumentIndex) { node.arguments !== undefined && node.arguments.length > argumentIndex) {
const arg = node.arguments[argumentIndex]; const arg = node.arguments[argumentIndex];
// No need to localize empty strings.
if (arg.type == 'Literal' && arg.value === '')
return;
if (!isLocalizationCall(arg)) { if (!isLocalizationCall(arg)) {
let order = ''; let order = '';
switch (argumentIndex) { switch (argumentIndex) {
...@@ -213,13 +246,13 @@ function checkFunctionArgument(functionName, argumentIndex, node, filePath, erro ...@@ -213,13 +246,13 @@ function checkFunctionArgument(functionName, argumentIndex, node, filePath, erro
* Check esprima node object that represents the AST of code * Check esprima node object that represents the AST of code
* to see if there is any localization error. * to see if there is any localization error.
*/ */
function analyzeNode(node, filePath, errors) { function analyzeNode(parentNode, node, filePath, errors) {
if (node === undefined || node === null) if (node === undefined || node === null)
return; return;
if (node instanceof Array) { if (node instanceof Array) {
for (const child of node) for (const child of node)
analyzeNode(child, filePath, errors); analyzeNode(node, child, filePath, errors);
return; return;
} }
...@@ -260,7 +293,7 @@ function analyzeNode(node, filePath, errors) { ...@@ -260,7 +293,7 @@ function analyzeNode(node, filePath, errors) {
break; break;
default: default:
// String concatenation to localization call(s) should be changed // String concatenation to localization call(s) should be changed
checkConcatenation(node, filePath, errors); checkConcatenation(parentNode, node, filePath, errors);
// 3rd argument to createInput() should be localized // 3rd argument to createInput() should be localized
checkFunctionArgument('createInput', 2, node, filePath, errors); checkFunctionArgument('createInput', 2, node, filePath, errors);
break; break;
...@@ -268,7 +301,7 @@ function analyzeNode(node, filePath, errors) { ...@@ -268,7 +301,7 @@ function analyzeNode(node, filePath, errors) {
for (const key of objKeys) { for (const key of objKeys) {
// recursively parse all the child nodes // recursively parse all the child nodes
analyzeNode(node[key], filePath, errors); analyzeNode(node, node[key], filePath, errors);
} }
} }
...@@ -282,7 +315,7 @@ async function auditFileForLocalizability(filePath, errors) { ...@@ -282,7 +315,7 @@ async function auditFileForLocalizability(filePath, errors) {
const relativeFilePath = getRelativeFilePathFromSrc(filePath); const relativeFilePath = getRelativeFilePathFromSrc(filePath);
for (const node of ast.body) for (const node of ast.body)
analyzeNode(node, relativeFilePath, errors); analyzeNode(undefined, node, relativeFilePath, errors);
} }
function shouldParseDirectory(directoryName) { function shouldParseDirectory(directoryName) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment