Reverse bookmark buttons and menus in RTL

chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm

Index: chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
index 1b2578c22d0859ea9f0723a867eddd9db124bf81..cbfb8356ed29a8c815518ce65132c5a90f36290c 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
@@ -9,6 +9,7 @@
 #include "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_constants.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_button.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_context_menu_cocoa_controller.h"
+#include "chrome/browser/ui/cocoa/l10n_util.h"
 #include "chrome/grit/generated_resources.h"
 #import "components/bookmarks/browser/bookmark_model.h"
 #include "content/public/browser/user_metrics.h"
@@ -23,15 +24,15 @@ using bookmarks::BookmarkNode;
 
 namespace {
 
-// Padding on the right side of the arrow icon.
-const int kHierarchyButtonRightPadding = 4;
+// Padding on the trailing side of the arrow icon.
+const int kHierarchyButtonTrailingPadding = 4;
 
-// Padding on the left side of the arrow icon.
-const int kHierarchyButtonLeftPadding = 11;
+// Padding on the leading side of the arrow icon.
+const int kHierarchyButtonLeadingPadding = 11;
 
 const int kIconTextSpacer = 4;
-const int kTextRightPadding = 4;
-const int kIconLeftPadding = 4;
+const int kTextTrailingPadding = 4;
+const int kIconLeadingPadding = 4;
 
 const int kDefaultFontSize = 12;
 
@@ -178,7 +179,7 @@ const int kDefaultFontSize = 12;
   [self setButtonType:NSMomentaryPushInButton];
   [self setShowsBorderOnlyWhileMouseInside:YES];
   [self setControlSize:NSSmallControlSize];
-  [self setAlignment:NSLeftTextAlignment];
+  [self setAlignment:NSNaturalTextAlignment];
   [self setFont:[NSFont systemFontOfSize:kDefaultFontSize]];
   [self setBordered:NO];
   [self setBezeled:NO];
@@ -220,12 +221,12 @@ const int kDefaultFontSize = 12;
                                            withString:@" "];
 
   if ([title length]) {
-    [self setImagePosition:NSImageLeft];
+    [self setImagePosition:cocoa_l10n_util::LeadingCellImagePosition()];
     [self setTitle:title];
   } else if ([self isFolderButtonCell]) {
     // Left-align icons for bookmarks within folders, regardless of whether
     // there is a title.
-    [self setImagePosition:NSImageLeft];
+    [self setImagePosition:cocoa_l10n_util::LeadingCellImagePosition()];
   } else {
     // For bookmarks without a title that aren't visible directly in the
     // bookmarks bar, squeeze things tighter by displaying only the image.
@@ -303,8 +304,11 @@ const int kDefaultFontSize = 12;
   drawFolderArrow_ = draw;
   if (draw && !arrowImage_) {
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    arrowImage_.reset(
-        [rb.GetNativeImageNamed(IDR_MENU_HIERARCHY_ARROW).ToNSImage() retain]);
+    NSImage* image =
+        rb.GetNativeImageNamed(IDR_MENU_HIERARCHY_ARROW).ToNSImage();
+    if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout())
+      image = cocoa_l10n_util::FlippedImage(image);
+    arrowImage_.reset([image retain]);
   }
 }
 
@@ -343,38 +347,59 @@ const int kDefaultFontSize = 12;
   NSSize cellSize = NSZeroSize;
   // Return the space needed to display the image and title, with a little
   // distance between them.
-  cellSize = NSMakeSize(kIconLeftPadding + [[self image] size].width,
+  cellSize = NSMakeSize(kIconLeadingPadding + [[self image] size].width,
                         bookmarks::kBookmarkButtonHeight);
   NSString* title = [self visibleTitle];
   if ([title length] > 0) {
     CGFloat textWidth =
         [title sizeWithAttributes:[self titleTextAttributes]].width;
     cellSize.width +=
-        kIconTextSpacer + std::ceil(textWidth) + kTextRightPadding;
+        kIconTextSpacer + std::ceil(textWidth) + kTextTrailingPadding;
   } else {
     // Make buttons without visible titles 20pts wide (18 plus padding).
-    cellSize.width += kIconLeftPadding;
+    cellSize.width += kIconLeadingPadding;
   }
 
   if (drawFolderArrow_) {
     cellSize.width += [arrowImage_ size].width +
-                      kHierarchyButtonLeftPadding +
-                      kHierarchyButtonRightPadding;
+                      kHierarchyButtonLeadingPadding +
+                      kHierarchyButtonTrailingPadding;
   }
+  // GradientButtonCell adds a 1px inset.
+  cellSize.width += 2;

[shrike, 2016/12/02 00:28:07]

The inset should be a value returned by an instance method, so that
BookmarkButtonCell can access it without needing to know anything about special
constants being used within in GradientButtonCell.

In GradientButtonCell the -inset method (or whatever you call it) should return
a kBlahBlahBlah - style constant rather than a raw int.

[lgrey, 2016/12/02 18:07:43]

Done. shrike@ PTAL

Turns out it's not actually a constant -- I got thrown by a local with a "k"
prefix in the relevant GradientButtonCell method (also renamed this for
posterity.)

   return cellSize;
 }
 
 - (NSRect)imageRectForBounds:(NSRect)theRect {
   NSRect imageRect = [super imageRectForBounds:theRect];
-  // Add a little space between the image and the button's left edge, but only
-  // if there's a visible title.
+  // Compensate by 1px since |theRect| is inset by 1px
+  // from the size calculated in |cellSize|.
   imageRect.origin.y -= 1;
-  imageRect.origin.x = kIconLeftPadding;
+  imageRect.origin.x =
+      cocoa_l10n_util::ShouldDoExperimentalRTLLayout()
+          ? NSMaxX(theRect) - kIconLeadingPadding - NSWidth(imageRect)
+          // Compensate for the inset
+          : kIconLeadingPadding + 1;
   return imageRect;
 }
 
-- (CGFloat)textStartXOffset {
-  return kIconLeftPadding + [[self image] size].width + kIconTextSpacer;
+- (NSRect)titleRectForBounds:(NSRect)theRect {
+  NSRect textRect = [super titleRectForBounds:theRect];
+  NSRect imageRect = [self imageRectForBounds:theRect];
+  if (cocoa_l10n_util::ShouldDoExperimentalRTLLayout()) {
+    textRect.origin.x = drawFolderArrow_
+                            ? [arrowImage_ size].width +
+                                  kHierarchyButtonTrailingPadding +
+                                  kTextTrailingPadding
+                            : kTextTrailingPadding;
+    // Compensate for the 1px inset
+    textRect.origin.x += 1;
+    textRect.size.width =
+        NSMinX(imageRect) - textRect.origin.x - kIconTextSpacer;
+  } else {
+    textRect.origin.x = NSMaxX(imageRect) + kIconTextSpacer;
+  }
+  return textRect;
 }
 
 - (void)drawFocusRingMaskWithFrame:(NSRect)cellFrame
@@ -406,8 +431,10 @@ const int kDefaultFontSize = 12;
     NSRect imageRect = NSZeroRect;
     imageRect.size = [arrowImage_ size];
     const CGFloat kArrowOffset = 1.0;  // Required for proper centering.
-    CGFloat dX =
-        NSWidth(cellFrame) - NSWidth(imageRect) - kHierarchyButtonRightPadding;
+    CGFloat dX = cocoa_l10n_util::ShouldDoExperimentalRTLLayout()
+                     ? kHierarchyButtonTrailingPadding
+                     : NSWidth(cellFrame) - NSWidth(imageRect) -
+                           kHierarchyButtonTrailingPadding;
     CGFloat dY = (NSHeight(cellFrame) / 2.0) - (NSHeight(imageRect) / 2.0) +
         kArrowOffset;
     NSRect drawRect = NSOffsetRect(imageRect, dX, dY);