Fix l10n for RTL languagues in the Request PIN dialog.

Fix l10n by no longer appending strings programmatically.

R=emaxx

      (e.g. smartcard), enter wrong PIN.

Bug: 1023360
Test: Enable RTL language, attempt to log in with PIN dialog
Change-Id: I9d4ffd3d1acef532d4899ef95a99d31af6fb682e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1926483Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Fabian Sommer <fabiansommer@google.com>
Cr-Commit-Position: refs/heads/master@{#720907}

chrome/app/chromeos_strings.grdp

@@ -3348,21 +3348,27 @@
   <message name="IDS_REQUEST_PIN_DIALOG_PROCESSING" desc="The text displayed while the certificate provider API is waiting for response from extension.">
     Processing request...
   </message>
-  <message name="IDS_REQUEST_PIN_DIALOG_INVALID_PIN_ERROR" desc="The error message displayed in the certificate provider PIN request dialog when an invalid PIN was entered.">
+  <message name="IDS_REQUEST_PIN_DIALOG_INVALID_PIN_ERROR" desc="The error message displayed in the certificate provider PIN request dialog when an invalid PIN was entered. Should end with a full stop since it can be chained with more text.">
     Invalid PIN.
   </message>
-  <message name="IDS_REQUEST_PIN_DIALOG_INVALID_PUK_ERROR" desc="The error message displayed in the certificate provider PIN request dialog when an invalid PUK was entered.">
+  <message name="IDS_REQUEST_PIN_DIALOG_INVALID_PUK_ERROR" desc="The error message displayed in the certificate provider PIN request dialog when an invalid PUK was entered. Should end with a full stop since it can be chained with more text.">
     Invalid PUK.
   </message>
   <message name="IDS_REQUEST_PIN_DIALOG_MAX_ATTEMPTS_EXCEEDED_ERROR" desc="The error message displayed in the certificate provider PIN request dialog when maximum allowed attempts exceeded.">
     Maximum allowed attempts exceeded.
   </message>
-  <message name="IDS_REQUEST_PIN_DIALOG_UNKNOWN_ERROR" desc="The error message displayed in the certificate provider PIN request dialog when unknown error occurred in extension code.">
+  <message name="IDS_REQUEST_PIN_DIALOG_UNKNOWN_ERROR" desc="The error message displayed in the certificate provider PIN request dialog when unknown error occurred in extension code. Should end with a full stop since it can be chained with more text.">
     Unknown error.
   </message>
   <message name="IDS_REQUEST_PIN_DIALOG_ATTEMPTS_LEFT" desc="The text displayed in the certificate provider PIN request dialog about the number of attempts left">
     Attempts left: <ph name="ATTEMPTS_LEFT">$1<ex>3</ex></ph>
   </message>
+  <message name="IDS_REQUEST_PIN_DIALOG_ERROR_RETRY" desc="The text displayed in the certificate provider PIN request dialog when the previous login attempt was unsuccessful, including the reason for the previous failure.">
+    <ph name="ERROR_MESSAGE">$1<ex>Invalid PIN.</ex></ph> Please try again.
+  </message>
+  <message name="IDS_REQUEST_PIN_DIALOG_ERROR_RETRY_ATTEMPTS" desc="The text displayed in the certificate provider PIN request dialog when the previous login attempt was unsuccessful but there are more attempts remaining. Includes the reason for the previous failure.">
+    <ph name="ERROR_MESSAGE">$1<ex>Invalid PIN.</ex></ph> Please try again. Attempts left: <ph name="ATTEMPTS_LEFT">$2<ex>3</ex></ph>
+  </message>
   <message name="IDS_REQUEST_PIN_DIALOG_PIN" desc="The Provider Identification Number abbreviation">
     PIN
   </message>

chrome/browser/chromeos/ui/request_pin_view.cc

@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/i18n/number_formatting.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chromeos/ui/passphrase_textfield.h"
@@ -129,7 +130,7 @@ void RequestPinView::SetDialogParameters(SecurityTokenPinCodeType code_type,
                                          int attempts_left,
                                          bool accept_input) {
   locked_ = false;
-  SetErrorMessage(error_label, attempts_left);
+  SetErrorMessage(error_label, attempts_left, accept_input);
   SetAcceptInput(accept_input);
 
   switch (code_type) {
@@ -225,7 +226,8 @@ void RequestPinView::SetAcceptInput(bool accept_input) {
 }
 
 void RequestPinView::SetErrorMessage(SecurityTokenPinErrorLabel error_label,
-                                     int attempts_left) {
+                                     int attempts_left,
+                                     bool accept_input) {
   base::string16 error_message;
   switch (error_label) {
     case SecurityTokenPinErrorLabel::kInvalidPin:
@@ -253,17 +255,25 @@ void RequestPinView::SetErrorMessage(SecurityTokenPinErrorLabel error_label,
       break;
   }
 
-  if (attempts_left >= 0) {
-    if (!error_message.empty())
-      error_message.append(base::ASCIIToUTF16(" "));
-    error_message.append(l10n_util::GetStringFUTF16(
-        IDS_REQUEST_PIN_DIALOG_ATTEMPTS_LEFT,
-        base::ASCIIToUTF16(std::to_string(attempts_left))));
+  base::string16 display_message;
+  if (!accept_input) {
+    display_message = error_message;
+  } else if (attempts_left == -1) {
+    display_message = l10n_util::GetStringFUTF16(
+        IDS_REQUEST_PIN_DIALOG_ERROR_RETRY, error_message);
+  } else if (error_message.empty()) {
+    display_message =
+        l10n_util::GetStringFUTF16(IDS_REQUEST_PIN_DIALOG_ATTEMPTS_LEFT,
+                                   base::FormatNumber(attempts_left));
+  } else {
+    display_message = l10n_util::GetStringFUTF16(
+        IDS_REQUEST_PIN_DIALOG_ERROR_RETRY_ATTEMPTS, error_message,
+        base::FormatNumber(attempts_left));
   }
 
   error_label_->SetVisible(true);
-  error_label_->SetText(error_message);
-  error_label_->SetTooltipText(error_message);
+  error_label_->SetText(display_message);
+  error_label_->SetTooltipText(display_message);
   error_label_->SetEnabledColor(gfx::kGoogleRed600);
   error_label_->SizeToPreferredSize();
   textfield_->SetInvalid(true);

chrome/browser/chromeos/ui/request_pin_view.h

@@ -93,7 +93,8 @@ class RequestPinView final : public views::DialogDelegateView,
   void Init();
   void SetAcceptInput(bool accept_input);
   void SetErrorMessage(SecurityTokenPinErrorLabel error_label,
-                       int attempts_left);
+                       int attempts_left,
+                       bool accept_input);
   // Updates the header text |header_label_| based on values from
   // |window_title_| and |code_type_|.
   void UpdateHeaderText();

chrome/browser/resources/chromeos/login/security_token_pin.html

@@ -27,26 +27,14 @@
       <div slot="footer">
         <div id="pinKeyboardContainer">
           <pin-keyboard id="pinKeyboard" enable-letters
-              has-error="[[hasError_(parameters, userEdited_)]]"
-              aria-label$="[[getAriaLabel_(locale, parameters, errorLabelId_,
-                                           userEdited_)]]"
+              has-error="[[isErrorLabelVisible_(errorLabelId_, userEdited_)]]"
+              aria-invalid$="[[isAriaInvalid_(parameters, userEdited_)]]"
               on-pin-change="onPinChange_" on-submit="onSubmit_"
               disabled="[[!canSubmit_]]">
-            <div id="errorContainer" problem
-                hidden="[[!hasError_(parameters, userEdited_)]]">
+            <div id="errorContainer" role="alert" problem
+                hidden$="[[!isLabelVisible_(parameters, userEdited_)]]">
               <iron-icon id="errorIcon" icon="cr:error-outline"></iron-icon>
-              <span hidden="[[!isErrorLabelVisible_(
-                                   errorLabelId_, userEdited_)]]">
-                [[i18nDynamic(locale, errorLabelId_)]]
-              </span>
-              <span hidden="[[!isErrorLabelVisible_(
-                                   errorLabelId_, userEdited_)]]">
-                [[i18nDynamic(locale, 'securityTokenPinDialogTryAgain')]]
-              </span>
-              <span hidden="[[!isAttemptsLeftVisible_(parameters)]]">
-                [[i18nDynamic(locale, 'securityTokenPinDialogAttemptsLeft',
-                              parameters.attemptsLeft)]]
-              </span>
+              [[getLabel_(locale, parameters, errorLabelId_, userEdited_)]]
             </div>
           </pin-keyboard>
         </div>

chrome/browser/resources/chromeos/login/security_token_pin.js

@@ -146,31 +146,28 @@ Polymer({
   },
 
   /**
-   * Returns whether an error message should be displayed to the user.
+   * Returns whether the error label should be shown.
    * @param {OobeTypes.SecurityTokenPinDialogParameters} parameters
    * @param {boolean} userEdited
    * @return {boolean}
    * @private
    */
-  hasError_: function(parameters, userEdited) {
-    if (!parameters)
-      return false;
-    if (parameters.attemptsLeft != -1)
-      return true;
-    return parameters.errorLabel !=
+  isErrorLabelVisible_: function(parameters, userEdited) {
+    return parameters &&
+        parameters.errorLabel !==
         OobeTypes.SecurityTokenPinDialogErrorType.NONE &&
         !userEdited;
   },
 
   /**
-   * Returns whether the error label should be shown.
-   * @param {string} errorLabelId
+   * Returns a string for a11y whether there is an error.
+   * @param {OobeTypes.SecurityTokenPinDialogParameters} parameters
    * @param {boolean} userEdited
-   * @return {boolean}
+   * @return {string}
    * @private
    */
-  isErrorLabelVisible_: function(errorLabelId, userEdited) {
-    return errorLabelId && !userEdited;
+  isAriaInvalid_: function(parameters, userEdited) {
+    return this.isErrorLabelVisible_(parameters, userEdited) ? 'true' : 'false';
   },
 
   /**
@@ -184,7 +181,19 @@ Polymer({
   },
 
   /**
-   * Returns the aria label to be used for the PIN input field.
+   * Returns whether there is a visible label for the PIN input field
+   * @param {OobeTypes.SecurityTokenPinDialogParameters} parameters
+   * @param {boolean} userEdited
+   * @return {boolean}
+   * @private
+   */
+  isLabelVisible_: function(parameters, userEdited) {
+    return this.isErrorLabelVisible_(parameters, userEdited) ||
+        this.isAttemptsLeftVisible_(parameters);
+  },
+
+  /**
+   * Returns the label to be used for the PIN input field.
    * @param {string} locale
    * @param {OobeTypes.SecurityTokenPinDialogParameters} parameters
    * @param {string} errorLabelId
@@ -192,18 +201,24 @@ Polymer({
    * @return {string}
    * @private
    */
-  getAriaLabel_: function(locale, parameters, errorLabelId, userEdited) {
-    var pieces = [];
-    if (this.isErrorLabelVisible_(errorLabelId, userEdited)) {
-      pieces.push(this.i18n(errorLabelId));
-      pieces.push(this.i18n('securityTokenPinDialogTryAgain'));
+  getLabel_: function(locale, parameters, errorLabelId, userEdited) {
+    if (!this.isLabelVisible_(parameters, userEdited)) {
+      return '';
+    }
+    if (!this.isErrorLabelVisible_(parameters, userEdited) &&
+        this.isAttemptsLeftVisible_(parameters)) {
+      return this.i18n(
+          'securityTokenPinDialogAttemptsLeft', parameters.attemptsLeft);
+    }
+    if (parameters && !parameters.enableUserInput) {
+      return this.i18n(errorLabelId);
     }
-    if (this.isAttemptsLeftVisible_(parameters)) {
-      pieces.push(this.i18n(
-          'securityTokenPinDialogAttemptsLeft', parameters.attemptsLeft));
+    if (!this.isAttemptsLeftVisible_(parameters)) {
+      return this.i18nRecursive(
+          locale, 'securityTokenPinDialogErrorRetry', errorLabelId);
     }
-    // Note: The language direction is not taken into account here, since the
-    // order of pieces follows the reading order.
-    return pieces.join(' ');
+    return this.i18n(
+        'securityTokenPinDialogErrorRetryAttempts', this.i18n(errorLabelId),
+        parameters.attemptsLeft);
   },
 });

chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc

@@ -671,6 +671,10 @@ void GaiaScreenHandler::DeclareLocalizedValues(
                IDS_SAML_SECURITY_TOKEN_PIN_DIALOG_TRY_AGAIN);
   builder->Add("securityTokenPinDialogAttemptsLeft",
                IDS_REQUEST_PIN_DIALOG_ATTEMPTS_LEFT);
+  builder->Add("securityTokenPinDialogErrorRetry",
+               IDS_REQUEST_PIN_DIALOG_ERROR_RETRY);
+  builder->Add("securityTokenPinDialogErrorRetryAttempts",
+               IDS_REQUEST_PIN_DIALOG_ERROR_RETRY_ATTEMPTS);
   builder->Add("securityTokenPinDialogUnknownError",
                IDS_REQUEST_PIN_DIALOG_UNKNOWN_ERROR);
   builder->Add("securityTokenPinDialogUnknownInvalidPin",