Fix RTL issues in javascript alerts.

chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.h"
 
 #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"
 #include "grit/generated_resources.h"
 #include "grit/ui_strings.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/base/ui_base_types.h"
 
 // Helper object that receives the notification that the dialog/sheet is
 // going away. Is responsible for cleaning itself up.
@@ skipping 115 matching lines @@
 
   // Show the modal dialog.
   alert_ = [helper_ alert];
   NSTextField* field = nil;
   if (text_field) {
     field = [helper_ textField];
     [field setStringValue:base::SysUTF16ToNSString(
         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) {
     NSButton* other = [alert_ addButtonWithTitle:other_button];
     [other setKeyEquivalent:@"\e"];
   }
   if (dialog_->display_suppress_checkbox()) {
     [alert_ setShowsSuppressionButton:YES];
     NSString* suppression_title = l10n_util::GetNSStringWithFixup(
         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() {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // JavaScriptAppModalDialogCocoa, NativeAppModalDialog implementation:
 
 int JavaScriptAppModalDialogCocoa::GetAppModalDialogButtons() const {
   // From the above, it is the case that if there is 1 button, it is always the
@@ skipping 45 matching lines @@
 
 ////////////////////////////////////////////////////////////////////////////////
 // NativeAppModalDialog, public:
 
 // static
 NativeAppModalDialog* NativeAppModalDialog::CreateNativeJavaScriptPrompt(
     JavaScriptAppModalDialog* dialog,
     gfx::NativeWindow parent_window) {
   return new JavaScriptAppModalDialogCocoa(dialog);
 }