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: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#647134}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7a329cb73b0b5d5cddb89a34fd7da6cb1a3c87dd

PRESUBMIT.py

@@ -202,7 +202,7 @@ def CheckChangeOnUpload(input_api, output_api):
     results = []
     results.extend(_CheckBuildGN(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(_CompileDevtoolsFrontend(input_api, output_api))
     results.extend(_CheckConvertSVGToPNGHashes(input_api, output_api))

front_end/audits2/Audits2Controller.js

@@ -97,8 +97,7 @@ Audits2.AuditController = class extends Common.Object {
     const inspectedURL = mainTarget && mainTarget.inspectedURL();
     if (inspectedURL && !/^(http|chrome-extension)/.test(inspectedURL)) {
       return Common.UIString(
-          'Can only audit HTTP/HTTPS pages and Chrome extensions. ' +
-          'Navigate to a different page to start an audit.');
+          'Can only audit HTTP/HTTPS pages and Chrome extensions. Navigate to a different page to start an audit.');
     }
 
     return null;
@@ -179,8 +178,7 @@ Audits2.AuditController = class extends Common.Object {
     let helpText = '';
     if (hasActiveServiceWorker) {
       helpText = Common.UIString(
-          'Multiple tabs are being controlled by the same service worker. ' +
-          'Close your other tabs on the same origin to audit this page.');
+          'Multiple tabs are being controlled by the same service worker. Close your other tabs on the same origin to audit this page.');
     } else if (!hasAtLeastOneCategory) {
       helpText = Common.UIString('At least one category must be selected.');
     } else if (unauditablePageMessage) {

front_end/audits2/Audits2StatusView.js

@@ -223,8 +223,7 @@ Audits2.StatusView = class {
     this._statusText.createChild('p').createTextChild(Common.UIString('Ah, sorry! We ran into an error.'));
     if (Audits2.StatusView.KnownBugPatterns.some(pattern => pattern.test(err.message))) {
       const message = Common.UIString(
-          'Try to navigate to the URL in a fresh Chrome profile without any other tabs or ' +
-          'extensions open and try again.');
+          'Try to navigate to the URL in a fresh Chrome profile without any other tabs or extensions open and try again.');
       this._statusText.createChild('p').createTextChild(message);
     } else {
       this._renderBugReportBody(err, this._inspectedURL);

front_end/coverage/CoverageView.js

@@ -76,8 +76,7 @@ Coverage.CoverageView = class extends UI.VBox {
     if (this._startWithReloadButton) {
       const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButtonForId('coverage.start-with-reload'));
       message = UI.formatLocalized(
-          'Click the record button %s to start capturing coverage.\n' +
-              'Click the reload button %s to reload and start capturing coverage.',
+          'Click the record button %s to start capturing coverage.\nClick the reload button %s to reload and start capturing coverage.',
           [recordButton, reloadButton]);
     } else {
       message = UI.formatLocalized('Click the record button %s to start capturing coverage.', [recordButton]);

front_end/network/ResourceWebSocketFrameView.js

@@ -115,9 +115,10 @@ Network.ResourceWebSocketFrameView = class extends UI.VBox {
    * @return {string}
    */
   static opCodeDescription(opCode, mask) {
-    const rawDescription = Network.ResourceWebSocketFrameView.opCodeDescriptions[opCode] || '';
-    const localizedDescription = Common.UIString(rawDescription);
-    return Common.UIString('%s (Opcode %d%s)', localizedDescription, opCode, (mask ? ', mask' : ''));
+    const localizedDescription = Network.ResourceWebSocketFrameView.opCodeDescriptions[opCode] || '';
+    if (mask)
+      return ls`${localizedDescription} (Opcode ${opCode}, mask)`;
+    return ls`${localizedDescription} (Opcode ${opCode})`;
   }
 
   /**
@@ -230,12 +231,12 @@ Network.ResourceWebSocketFrameView.OpCodes = {
 Network.ResourceWebSocketFrameView.opCodeDescriptions = (function() {
   const opCodes = Network.ResourceWebSocketFrameView.OpCodes;
   const map = [];
-  map[opCodes.ContinuationFrame] = 'Continuation Frame';
-  map[opCodes.TextFrame] = 'Text Message';
-  map[opCodes.BinaryFrame] = 'Binary Message';
-  map[opCodes.ContinuationFrame] = 'Connection Close Message';
-  map[opCodes.PingFrame] = 'Ping Message';
-  map[opCodes.PongFrame] = 'Pong Message';
+  map[opCodes.ContinuationFrame] = ls`Continuation Frame`;
+  map[opCodes.TextFrame] = ls`Text Message`;
+  map[opCodes.BinaryFrame] = ls`Binary Message`;
+  map[opCodes.ContinuationFrame] = ls`Connection Close Message`;
+  map[opCodes.PingFrame] = ls`Ping Message`;
+  map[opCodes.PongFrame] = ls`Pong Message`;
   return map;
 })();
 
@@ -272,7 +273,7 @@ Network.ResourceWebSocketFrameNode = class extends DataGrid.SortableDataGridNode
 
     } else if (frame.opCode === Network.ResourceWebSocketFrameView.OpCodes.BinaryFrame) {
       length = Number.bytesToString(base64ToSize(frame.text));
-      description = 'Binary Message';
+      description = Network.ResourceWebSocketFrameView.opCodeDescriptions[frame.opCode];
 
     } else {
       dataText = description;

front_end/sdk/NetworkManager.js

@@ -858,8 +858,7 @@ SDK.NetworkDispatcher = class {
 
     if (shouldReportCorbBlocking) {
       const message = Common.UIString(
-          `Cross-Origin Read Blocking (CORB) blocked cross-origin response %s with MIME type %s. ` +
-              `See https://www.chromestatus.com/feature/5629709824032768 for more details.`,
+          `Cross-Origin Read Blocking (CORB) blocked cross-origin response %s with MIME type %s. See https://www.chromestatus.com/feature/5629709824032768 for more details.`,
           networkRequest.url(), networkRequest.mimeType);
       this._manager.dispatchEventToListeners(
           SDK.NetworkManager.Events.MessageGenerated,
@@ -868,10 +867,18 @@ SDK.NetworkDispatcher = class {
 
     if (Common.moduleSetting('monitoringXHREnabled').get() &&
         networkRequest.resourceType().category() === Common.resourceCategories.XHR) {
-      const message = Common.UIString(
-          (networkRequest.failed || networkRequest.hasErrorStatusCode()) ? '%s failed loading: %s "%s".' :
-                                                                           '%s finished loading: %s "%s".',
-          networkRequest.resourceType().title(), networkRequest.requestMethod, networkRequest.url());
+      let message;
+      const failedToLoad = networkRequest.failed || networkRequest.hasErrorStatusCode();
+      if (failedToLoad) {
+        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(
           SDK.NetworkManager.Events.MessageGenerated,
           {message: message, requestId: networkRequest.requestId(), warning: false});

front_end/sources/GoToLineQuickOpen.js

@@ -29,11 +29,9 @@ Sources.GoToLineQuickOpen = class extends QuickOpen.FilteredListWidget.Provider
     const position = this._parsePosition(query);
     if (!position)
       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)
-      text += Common.UIString(' and column ') + position.column;
-    text += '.';
-    return text;
+      return ls`Go to line ${position.line} and column ${position.column}.`;
+    return ls`Go to line ${position.line}.`;
   }
 
   /**

front_end/sources/SourcesPanel.js

@@ -460,8 +460,7 @@ Sources.SourcesPanel = class extends UI.Panel {
   _pauseOnExceptionEnabledChanged() {
     const enabled = Common.moduleSetting('pauseOnExceptionEnabled').get();
     this._pauseOnExceptionButton.setToggled(enabled);
-    this._pauseOnExceptionButton.setTitle(
-        Common.UIString(enabled ? 'Don\'t pause on exceptions' : 'Pause on exceptions'));
+    this._pauseOnExceptionButton.setTitle(enabled ? ls`Don't pause on exceptions` : ls`Pause on exceptions`);
     this._debugToolbarDrawer.classList.toggle('expanded', enabled);
   }
 

front_end/sources/ThreadsSidebarPane.js

@@ -45,7 +45,7 @@ Sources.ThreadsSidebarPane = class extends UI.VBox {
     }
 
     function updatePausedState() {
-      pausedState.textContent = Common.UIString(debuggerModel.isPaused() ? 'paused' : '');
+      pausedState.textContent = debuggerModel.isPaused() ? ls`paused` : '';
     }
 
     /**

front_end/timeline/TimelinePanel.js

@@ -655,13 +655,11 @@ Timeline.TimelinePanel = class extends UI.Panel {
     const reloadButton = UI.createInlineButton(UI.Toolbar.createActionButtonForId('timeline.record-reload'));
 
     centered.createChild('p').appendChild(UI.formatLocalized(
-        'Click the record button %s or hit %s to start a new recording.\n' +
-            'Click the reload button %s or hit %s to record the page load.',
+        '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.',
         [recordButton, recordKey, reloadButton, reloadKey]));
 
     centered.createChild('p').appendChild(UI.formatLocalized(
-        'After recording, select an area of interest in the overview by dragging.\n' +
-        'Then, zoom and pan the timeline with the mousewheel or %s keys.\n%s',
+        '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',
         [navigateNode, learnMoreNode]));
 
     this._landingPage.show(this._statusPaneContainer);

front_end/timeline/TimelineUIUtils.js

@@ -556,10 +556,13 @@ Timeline.TimelineUIUtils = class {
         break;
       }
       case recordType.ParseHTML: {
+        const startLine = event.args['beginData']['startLine'];
         const endLine = event.args['endData'] && event.args['endData']['endLine'];
         const url = Bindings.displayNameForURL(event.args['beginData']['url']);
-        detailsText = Common.UIString(
-            '%s [%s\u2026%s]', url, event.args['beginData']['startLine'] + 1, endLine >= 0 ? endLine + 1 : '');
+        if (endLine >= 0)
+          detailsText = Common.UIString('%s [%s\u2026%s]', url, startLine + 1, endLine + 1);
+        else
+          detailsText = Common.UIString('%s [%s\u2026]', url, startLine + 1);
         break;
       }
       case recordType.CompileModule:

scripts/check_localizability.js

@@ -153,19 +153,48 @@ function getLocation(node) {
   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.
+ * 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) {
-  if (node !== undefined && node.type === esprimaTypes.BI_EXPR && node.operator === '+') {
-    const code = escodegen.generate(node);
-    if (isLocalizationCall(node.left) || isLocalizationCall(node.right)) {
-      addError(
-          `${filePath}${getLocation(node)}: string concatenation should be changed to variable substitution with ls: ${
-              code}`,
-          errors);
-    } else {
-      [node.left, node.right].forEach(node => checkConcatenation(node, filePath, errors));
+function checkConcatenation(parentNode, node, filePath, errors) {
+  function isWord(node) {
+    return (node.type === 'Literal' && !!node.value.match(/[a-z]/i));
+  }
+  function isConcatenation(node) {
+    return (node !== undefined && node.type === esprimaTypes.BI_EXPR && node.operator === '+');
+  }
+
+  if (isConcatenation(parentNode))
+    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
   if (node !== undefined && node.type === esprimaTypes.CALL_EXPR && verifyFunctionCallee(node.callee, functionName) &&
       node.arguments !== undefined && node.arguments.length > argumentIndex) {
     const arg = node.arguments[argumentIndex];
+    // No need to localize empty strings.
+    if (arg.type == 'Literal' && arg.value === '')
+      return;
+
     if (!isLocalizationCall(arg)) {
       let order = '';
       switch (argumentIndex) {
@@ -213,13 +246,13 @@ function checkFunctionArgument(functionName, argumentIndex, node, filePath, erro
  * Check esprima node object that represents the AST of code
  * to see if there is any localization error.
  */
-function analyzeNode(node, filePath, errors) {
+function analyzeNode(parentNode, node, filePath, errors) {
   if (node === undefined || node === null)
     return;
 
   if (node instanceof Array) {
     for (const child of node)
-      analyzeNode(child, filePath, errors);
+      analyzeNode(node, child, filePath, errors);
 
     return;
   }
@@ -260,7 +293,7 @@ function analyzeNode(node, filePath, errors) {
       break;
     default:
       // 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
       checkFunctionArgument('createInput', 2, node, filePath, errors);
       break;
@@ -268,7 +301,7 @@ function analyzeNode(node, filePath, errors) {
 
   for (const key of objKeys) {
     // 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) {
 
   const relativeFilePath = getRelativeFilePathFromSrc(filePath);
   for (const node of ast.body)
-    analyzeNode(node, relativeFilePath, errors);
+    analyzeNode(undefined, node, relativeFilePath, errors);
 }
 
 function shouldParseDirectory(directoryName) {