[rtl] Fix tail-fading of text for bookmark buttons.

BUG=388084

Review URL: https://codereview.chromium.org/1561123003

Cr-Commit-Position: refs/heads/master@{#368719}

5 files changed, 57 insertions, 47 deletions

diff --git a/chrome/browser/ui/views/profiles/profile_chooser_view.cc b/chrome/browser/ui/views/profiles/profile_chooser_view.cc
index 2c21f47..da130fc 100644
--- a/chrome/browser/ui/views/profiles/profile_chooser_view.cc
+++ b/chrome/browser/ui/views/profiles/profile_chooser_view.cc
@@ -264,14 +264,20 @@ class RightAlignedIconLabelButton : public views::LabelButton {
   RightAlignedIconLabelButton(views::ButtonListener* listener,
                               const base::string16& text)
       : views::LabelButton(listener, text) {
+    SetHorizontalAlignment(gfx::ALIGN_RIGHT);
+    label()->SetHorizontalAlignment(gfx::ALIGN_CENTER);
   }
 
  protected:
   void Layout() override {
-    // This layout trick keeps the text left-aligned and the icon right-aligned.
-    SetHorizontalAlignment(gfx::ALIGN_RIGHT);
     views::LabelButton::Layout();
-    label()->SetHorizontalAlignment(gfx::ALIGN_CENTER);
+
+    // Keep the text centered and the icon right-aligned by stretching the label
+    // to take up more of the content area and centering its contents.
+    gfx::Rect content_bounds = GetContentsBounds();
+    gfx::Rect label_bounds = label()->bounds();
+    label_bounds.Inset(content_bounds.x() - label_bounds.x(), 0, 0, 0);
+    label()->SetBoundsRect(label_bounds);
   }
 
  private:
@@ -388,7 +394,6 @@ class EditableProfileName : public views::View,
     const gfx::FontList& medium_font_list =
         rb->GetFontList(ui::ResourceBundle::MediumFont);
     button_->SetFontList(medium_font_list);
-    button_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
 
     if (!is_editing_allowed) {
       button_->SetBorder(views::Border::CreateEmptyBorder(2, 0, 2, 0));
diff --git a/ui/views/controls/button/label_button.cc b/ui/views/controls/button/label_button.cc
index 8480fd2..06bbd26 100644
--- a/ui/views/controls/button/label_button.cc
+++ b/ui/views/controls/button/label_button.cc
@@ -72,7 +72,8 @@ LabelButton::LabelButton(ButtonListener* listener, const base::string16& text)
       is_default_(false),
       style_(STYLE_TEXTBUTTON),
       border_is_themed_border_(true),
-      image_label_spacing_(kSpacing) {
+      image_label_spacing_(kSpacing),
+      horizontal_alignment_(gfx::ALIGN_LEFT) {
   SetAnimationDuration(kHoverAnimationDurationMs);
   SetText(text);
 
@@ -82,7 +83,7 @@ LabelButton::LabelButton(ButtonListener* listener, const base::string16& text)
   AddChildView(label_);
   label_->SetFontList(cached_normal_font_list_);
   label_->SetAutoColorReadabilityEnabled(false);
-  label_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  label_->SetHorizontalAlignment(gfx::ALIGN_TO_HEAD);
 
   // Inset the button focus rect from the actual border; roughly match Windows.
   SetFocusPainter(Painter::CreateDashedFocusPainterWithInsets(gfx::Insets(
@@ -153,12 +154,9 @@ void LabelButton::SetElideBehavior(gfx::ElideBehavior elide_behavior) {
   label_->SetElideBehavior(elide_behavior);
 }
 
-gfx::HorizontalAlignment LabelButton::GetHorizontalAlignment() const {
-  return label_->horizontal_alignment();
-}
-
 void LabelButton::SetHorizontalAlignment(gfx::HorizontalAlignment alignment) {
-  label_->SetHorizontalAlignment(alignment);
+  DCHECK_NE(gfx::ALIGN_TO_HEAD, alignment);
+  horizontal_alignment_ = alignment;
   InvalidateLayout();
 }
 
@@ -196,7 +194,7 @@ void LabelButton::SetStyle(ButtonStyle style) {
   style_ = style;
 
   SetFocusPainter(nullptr);
-  label_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
+  SetHorizontalAlignment(gfx::ALIGN_CENTER);
   SetFocusable(true);
   SetMinSize(gfx::Size(70, 33));
 
@@ -279,14 +277,11 @@ int LabelButton::GetHeightForWidth(int w) const {
 }
 
 void LabelButton::Layout() {
-  gfx::HorizontalAlignment adjusted_alignment = GetHorizontalAlignment();
-  if (base::i18n::IsRTL() && adjusted_alignment != gfx::ALIGN_CENTER)
-    adjusted_alignment = (adjusted_alignment == gfx::ALIGN_LEFT) ?
-        gfx::ALIGN_RIGHT : gfx::ALIGN_LEFT;
-
   // By default, GetChildAreaBounds() ignores the top and bottom border, but we
   // want the image to respect it.
   gfx::Rect child_area(GetChildAreaBounds());
+  // The space that the label can use. Its actual bounds may be smaller if the
+  // label is short.
   gfx::Rect label_area(child_area);
 
   gfx::Insets insets(GetInsets());
@@ -297,40 +292,43 @@ void LabelButton::Layout() {
   gfx::Size image_size(image_->GetPreferredSize());
   image_size.SetToMin(child_area.size());
 
-  // The label takes any remaining width after sizing the image, unless both
-  // views are centered. In that case, using the tighter preferred label width
-  // avoids wasted space within the label that would look like awkward padding.
-  gfx::Size label_size(label_area.size());
-  if (!image_size.IsEmpty() && !label_size.IsEmpty()) {
-    label_size.set_width(std::max(child_area.width() -
-        image_size.width() - image_label_spacing_, 0));
-    if (adjusted_alignment == gfx::ALIGN_CENTER) {
-      // Ensure multi-line labels paired with images use their available width.
-      label_size.set_width(
-          std::min(label_size.width(), label_->GetPreferredSize().width()));
-    }
+  if (!image_size.IsEmpty()) {
+    int image_space = image_size.width() + image_label_spacing_;
+    if (horizontal_alignment_ == gfx::ALIGN_RIGHT)
+      label_area.Inset(0, 0, image_space, 0);
+    else
+      label_area.Inset(image_space, 0, 0, 0);
   }
 
+  gfx::Size label_size(
+      std::min(label_area.width(), label_->GetPreferredSize().width()),
+      label_area.height());
+
   gfx::Point image_origin(child_area.origin());
   image_origin.Offset(0, (child_area.height() - image_size.height()) / 2);
-  if (adjusted_alignment == gfx::ALIGN_CENTER) {
+  if (horizontal_alignment_ == gfx::ALIGN_CENTER) {
     const int spacing = (image_size.width() > 0 && label_size.width() > 0) ?
         image_label_spacing_ : 0;
     const int total_width = image_size.width() + label_size.width() +
         spacing;
     image_origin.Offset((child_area.width() - total_width) / 2, 0);
-  } else if (adjusted_alignment == gfx::ALIGN_RIGHT) {
+  } else if (horizontal_alignment_ == gfx::ALIGN_RIGHT) {
     image_origin.Offset(child_area.width() - image_size.width(), 0);
   }
+  image_->SetBoundsRect(gfx::Rect(image_origin, image_size));
 
-  gfx::Point label_origin(label_area.origin());
-  if (!image_size.IsEmpty() && adjusted_alignment != gfx::ALIGN_RIGHT) {
-    label_origin.set_x(image_origin.x() + image_size.width() +
-        image_label_spacing_);
+  gfx::Rect label_bounds = label_area;
+  if (label_area.width() == label_size.width()) {
+    // Label takes up the whole area.
+  } else if (horizontal_alignment_ == gfx::ALIGN_CENTER) {
+    label_bounds.ClampToCenteredSize(label_size);
+  } else {
+    label_bounds.set_size(label_size);
+    if (horizontal_alignment_ == gfx::ALIGN_RIGHT)
+      label_bounds.Offset(label_area.width() - label_size.width(), 0);
   }
 
-  image_->SetBoundsRect(gfx::Rect(image_origin, image_size));
-  label_->SetBoundsRect(gfx::Rect(label_origin, label_size));
+  label_->SetBoundsRect(label_bounds);
   CustomButton::Layout();
 }
 
diff --git a/ui/views/controls/button/label_button.h b/ui/views/controls/button/label_button.h
index a086fe8..fb4846f 100644
--- a/ui/views/controls/button/label_button.h
+++ b/ui/views/controls/button/label_button.h
@@ -64,9 +64,8 @@ class VIEWS_EXPORT LabelButton : public CustomButton,
   // Sets the elide behavior of this button.
   void SetElideBehavior(gfx::ElideBehavior elide_behavior);
 
-  // Gets or sets the horizontal alignment used for the button; reversed in RTL.
-  // The optional image will lead the text, unless the button is right-aligned.
-  gfx::HorizontalAlignment GetHorizontalAlignment() const;
+  // Sets the horizontal alignment used for the button; reversed in RTL. The
+  // optional image will lead the text, unless the button is right-aligned.
   void SetHorizontalAlignment(gfx::HorizontalAlignment alignment);
 
   // Call SetMinSize(gfx::Size()) to clear the monotonically increasing size.
@@ -196,6 +195,12 @@ class VIEWS_EXPORT LabelButton : public CustomButton,
   // Spacing between the image and the text.
   int image_label_spacing_;
 
+  // Alignment of the button. This can be different from the alignment of the
+  // text; for example, the label may be set to ALIGN_TO_HEAD (alignment matches
+  // text direction) while |this| is laid out as ALIGN_LEFT (alignment matches
+  // UI direction).
+  gfx::HorizontalAlignment horizontal_alignment_;
+
   scoped_ptr<Painter> focus_painter_;
 
   DISALLOW_COPY_AND_ASSIGN(LabelButton);
diff --git a/ui/views/controls/button/label_button_unittest.cc b/ui/views/controls/button/label_button_unittest.cc
index 0bfb7d9..f0b03045 100644
--- a/ui/views/controls/button/label_button_unittest.cc
+++ b/ui/views/controls/button/label_button_unittest.cc
@@ -73,7 +73,6 @@ TEST_F(LabelButtonTest, Init) {
   EXPECT_EQ(ui::AX_ROLE_BUTTON, accessible_state.role);
   EXPECT_EQ(text, accessible_state.name);
 
-  EXPECT_EQ(gfx::ALIGN_LEFT, button.GetHorizontalAlignment());
   EXPECT_FALSE(button.is_default());
   EXPECT_EQ(button.style(), Button::STYLE_TEXTBUTTON);
   EXPECT_EQ(Button::STATE_NORMAL, button.state());
@@ -194,17 +193,20 @@ TEST_F(LabelButtonTest, LabelAndImage) {
 
   // Layout and ensure the image is left of the label except for ALIGN_RIGHT.
   // (A proper parent view or layout manager would Layout on its invalidations).
-  button_->SetSize(button_->GetPreferredSize());
+  // Also make sure CENTER alignment moves the label compared to LEFT alignment.
+  gfx::Size button_size = button_->GetPreferredSize();
+  button_size.Enlarge(50, 0);
+  button_->SetSize(button_size);
   button_->Layout();
-  EXPECT_EQ(gfx::ALIGN_LEFT, button_->GetHorizontalAlignment());
   EXPECT_LT(button_->image_->bounds().right(), button_->label_->bounds().x());
+  int left_align_label_midpoint = button_->label_->bounds().CenterPoint().x();
   button_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
   button_->Layout();
-  EXPECT_EQ(gfx::ALIGN_CENTER, button_->GetHorizontalAlignment());
   EXPECT_LT(button_->image_->bounds().right(), button_->label_->bounds().x());
+  int center_align_label_midpoint = button_->label_->bounds().CenterPoint().x();
+  EXPECT_LT(left_align_label_midpoint, center_align_label_midpoint);
   button_->SetHorizontalAlignment(gfx::ALIGN_RIGHT);
   button_->Layout();
-  EXPECT_EQ(gfx::ALIGN_RIGHT, button_->GetHorizontalAlignment());
   EXPECT_LT(button_->label_->bounds().right(), button_->image_->bounds().x());
 
   button_->SetText(base::string16());
diff --git a/ui/views/examples/button_example.cc b/ui/views/examples/button_example.cc
index a574a9c..5b0b1e5 100644
--- a/ui/views/examples/button_example.cc
+++ b/ui/views/examples/button_example.cc
@@ -93,9 +93,9 @@ void ButtonExample::LabelButtonPressed(const ui::Event& event) {
           label_button_->GetImage(Button::STATE_NORMAL).isNull() ?
           *icon_ : gfx::ImageSkia());
     } else {
+      static int alignment = 0;
       label_button_->SetHorizontalAlignment(
-          static_cast<gfx::HorizontalAlignment>(
-              (label_button_->GetHorizontalAlignment() + 1) % 3));
+          static_cast<gfx::HorizontalAlignment>(++alignment % 3));
     }
   } else if (event.IsShiftDown()) {
     if (event.IsAltDown()) {