Fix RTL issues in javascript alerts.

chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm

Index: chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm b/chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm
index f92de8c0e4af49483756c2a0d11e3dde05255575..b644931e7ae850717901651a36124f0380037e16 100644
--- a/chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm
+++ b/chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm
@@ -6,7 +6,9 @@
 
 #import <Cocoa/Cocoa.h>
 
+#include "base/i18n/rtl.h"
 #include "base/logging.h"
+#include "base/mac/foundation_util.h"

[Mark Mentovai, 2013/05/16 22:34:27]

#import me.

 #include "base/strings/sys_string_conversions.h"
 #import "chrome/browser/chrome_browser_application_mac.h"
 #include "chrome/browser/ui/app_modal_dialogs/javascript_app_modal_dialog.h"
@@ -142,7 +144,8 @@ JavaScriptAppModalDialogCocoa::JavaScriptAppModalDialogCocoa(
         dialog_->default_prompt_text())];
   }
   [alert_ setDelegate:helper_];
-  [alert_ setInformativeText:base::SysUTF16ToNSString(dialog_->message_text())];
+  NSString* informativeText = base::SysUTF16ToNSString(dialog_->message_text());

[Mark Mentovai, 2013/05/16 22:34:27]

Outside of @interface, @implementation, etc., name_like_this, notLikeThis.

+  [alert_ setInformativeText:informativeText];
   [alert_ setMessageText:base::SysUTF16ToNSString(dialog_->title())];
   [alert_ addButtonWithTitle:default_button];
   if (!one_button) {
@@ -155,6 +158,56 @@ JavaScriptAppModalDialogCocoa::JavaScriptAppModalDialogCocoa(
         IDS_JAVASCRIPT_MESSAGEBOX_SUPPRESS_OPTION);
     [[alert_ suppressionButton] setTitle:suppression_title];
   }
+
+  // Fix RTL dialogs.

[Mark Mentovai, 2013/05/16 22:34:27]

Can you avoid doing this if it’s not needed? Gate this whole block on
base::i18n::StringContainsStrongRTLChars(dialog_->message_text()).

+  //
+  // Mac OS X will always display an NSAlert's informative text as LTR. A
+  // workaround is to manually set the informative text as an attributed string
+  // in the implementing NSTextField. This is a basic correctness issue.
+  //
+  // In addition, for readability, the overall alignment is set based on the
+  // directionality of the first strongly-directional character.
+  //
+  // See http://crbug.com/70806 for more details.
+
+  // Force layout of the dialog. NSAlert leaves its dialog alone once laid out;
+  // if this is not done then all the modifications that are to come will be
+  // un-done when the dialog is finally displayed.
+  [alert_ layout];
+
+  // Locate the NSTextField that implements the informative text. This is
+  // actually available as the ivar |_informationField| of the NSAlert, but it
+  // is safer (and more forward-compatible) to search for it in the subviews.
+  NSTextField* informativeTextField = nil;
+  for (NSView* view in [[[alert_ window] contentView] subviews]) {
+    NSTextField* textField = base::mac::ObjCCast<NSTextField>(view);
+    if (!textField)

[Mark Mentovai, 2013/05/16 22:34:27]

This check is unnecessary.

+      continue;
+
+    if (![[textField stringValue] isEqualTo:informativeText])
+      continue;
+
+    informativeTextField = textField;

[Mark Mentovai, 2013/05/16 22:34:27]

break here?

Since you’ve removed one of the “continue”s, the loop would probably work better
if you structured it as

  for each view {
    text field = cast(view)
    if (text field's string value is equal) {
      remember this field
      break
    }
  }

+  }
+
+  if (informativeTextField) {

[Mark Mentovai, 2013/05/16 22:34:27]

DCHECK(informativeTextField) ?

+    base::i18n::TextDirection direction =
+        base::i18n::GetFirstStrongCharacterDirection(dialog_->message_text());
+    scoped_nsobject<NSMutableParagraphStyle> alignment(
+      [[NSParagraphStyle defaultParagraphStyle] mutableCopy]);

[Mark Mentovai, 2013/05/16 22:34:27]

Nit: continuation line indent gets another 2 spaces.

+    if (direction == base::i18n::RIGHT_TO_LEFT)
+      [alignment setAlignment:NSRightTextAlignment];

[Mark Mentovai, 2013/05/16 22:34:27]

Ideas: Ternary? Or avoid having an alignment attribute altogether if it’s LTR?

[Avi (use Gerrit), 2013/05/16 23:16:20]

Ternary. At this point I'm a little paranoid and just want to set things.

+    else
+      [alignment setAlignment:NSLeftTextAlignment];
+
+    NSDictionary* alignmentAttributes =
+        @{ NSParagraphStyleAttributeName : alignment };
+    scoped_nsobject<NSAttributedString> attrString(
+        [[NSAttributedString alloc] initWithString:informativeText
+                                        attributes:alignmentAttributes]);
+
+    [informativeTextField setAttributedStringValue:attrString];
+  }
 }
 
 JavaScriptAppModalDialogCocoa::~JavaScriptAppModalDialogCocoa() {