MacViews: Tweak kDragToEndIfOutsideVerticalBounds for single line fields.

ui/views/controls/textfield/textfield_unittest.cc

 // Copyright 2014 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 "ui/views/controls/textfield/textfield.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 1546 matching lines @@
   EXPECT_EQ(text_right, textfield_->GetSelectedText());
 
   // Check that dragging from beyond the text length works too.
   MoveMouseTo(end_point);
   PressLeftMouseButton();
   DragMouseTo(gfx::Point(0, cursor_y));
   ReleaseLeftMouseButton();
   EXPECT_EQ(textfield_->text(), textfield_->GetSelectedText());
 }
 
-// This test checks that dragging above the textfield selects to the beginning
-// and dragging below the textfield selects to the end, but only on platforms
-// where that is the expected behavior.
+// Ensures dragging above or below the textfield extends a selection to either
+// end, depending on the relative x offsets of the text and mouse cursors.
 TEST_F(TextfieldTest, DragUpOrDownSelectsToEnd) {
   InitTextfield();
   textfield_->SetText(ASCIIToUTF16("hello world"));
-  const base::string16 expected_up = base::ASCIIToUTF16(
+  const base::string16 expected_left = base::ASCIIToUTF16(
       gfx::RenderText::kDragToEndIfOutsideVerticalBounds ? "hello" : "lo");
-  const base::string16 expected_down = base::ASCIIToUTF16(
+  const base::string16 expected_right = base::ASCIIToUTF16(
       gfx::RenderText::kDragToEndIfOutsideVerticalBounds ? " world" : " w");
   const int kStartX = GetCursorPositionX(5);

[msw, 2017/05/25 18:11:56]

optional nit: inline this?

[tapted, 2017/05/26 06:53:40]

Done.

-  const int kDownX = GetCursorPositionX(7);
-  const int kUpX = GetCursorPositionX(3);
-  gfx::Point start_point(kStartX, GetCursorYForTesting());
-  gfx::Point down_point(kDownX, 500);
-  gfx::Point up_point(kUpX, -500);
+  const int kRightX = GetCursorPositionX(7);

[msw, 2017/05/25 18:11:56]

optional nit: unix_hacker naming style to match other consts here?

[tapted, 2017/05/26 06:53:40]

Done.

+  const int kLeftX = GetCursorPositionX(3);
+  const gfx::Point start_point(kStartX, GetCursorYForTesting());
+  const gfx::Point end_point[] = {
+      {kLeftX, -500}, {kRightX, -500}, {kRightX, 500}, {kLeftX, 500}};
 
-  MoveMouseTo(start_point);
-  PressLeftMouseButton();
-  DragMouseTo(up_point);
-  ReleaseLeftMouseButton();
-  EXPECT_EQ(textfield_->GetSelectedText(), expected_up);
+  auto SelectionAfterDrag = [&, this](const gfx::Point& end) -> base::string16 {

[msw, 2017/05/25 18:11:56]

nifty!

+    MoveMouseTo(start_point);
+    PressLeftMouseButton();
+    DragMouseTo(end);
+    ReleaseLeftMouseButton();
+    base::string16 selection = textfield_->GetSelectedText();
 
-  // Click at |up_point|. This is important because drags do not count as clicks
-  // for the purpose of double-click detection, so if this test doesn't click
-  // somewhere other than |start_point| before the code below runs, the second
-  // click at |start_point| will be interpreted as a double-click instead of the
-  // start of a drag.
-  ClickLeftMouseButton();
+    // Click at |end|. This is important because drags do not count as clicks

[msw, 2017/05/25 18:11:56]

Since these all have the same start point, we should be able to put the
MoveMouseTo(start_point); and PressLeftMouseButton(); before the lambda
[executions] and then just make the lambda body { DragMouseTo(end); return
textfield_->GetSelectedText(); } (without any mouse release, press, long
comment, or restart of move and press) We could also just match what the label
tests do and repeat the commands inline:
  DragMouseTo(gfx::Point(kLeftX, -500));  // NW
  EXPECT_EQ(expected_left, textfield_->GetSelectedText());
  DragMouseTo...
  EXPECT_EQ...

[tapted, 2017/05/26 06:53:40]

Done.

+    // for the purpose of double-click detection, so if this test doesn't click
+    // somewhere other than |start_point| before the code below runs, the second
+    // click at |start_point| will be interpreted as a double-click instead of
+    // the start of a drag.
+    ClickLeftMouseButton();
+    return selection;
+  };
 
-  MoveMouseTo(start_point);
-  PressLeftMouseButton();
-  DragMouseTo(down_point);
-  ReleaseLeftMouseButton();
-  EXPECT_EQ(textfield_->GetSelectedText(), expected_down);
+  enum { NW, NE, SE, SW };
+  EXPECT_EQ(expected_left, SelectionAfterDrag(end_point[NW]));

[msw, 2017/05/25 18:11:56]

nit: I'd rather have the coordinates inlined here and just comment " // NW" or
similar, then get rid of the enum and |end_point| array.

[tapted, 2017/05/26 06:53:40]

Done.

+  EXPECT_EQ(expected_right, SelectionAfterDrag(end_point[NE]));
+  EXPECT_EQ(expected_right, SelectionAfterDrag(end_point[SE]));
+  EXPECT_EQ(expected_left, SelectionAfterDrag(end_point[SW]));
 }
 
 #if defined(OS_WIN)
 TEST_F(TextfieldTest, DragAndDrop_AcceptDrop) {
   InitTextfield();
   textfield_->SetText(ASCIIToUTF16("hello world"));
 
   ui::OSExchangeData data;
   base::string16 string(ASCIIToUTF16("string "));
   data.SetString(string);
@@ skipping 1550 matching lines @@
   InitTextfield();
 
   textfield_->SetCursorEnabled(false);
   EXPECT_FALSE(test_api_->IsCursorVisible());
 
   textfield_->SetCursorEnabled(true);
   EXPECT_TRUE(test_api_->IsCursorVisible());
 }
 
 }  // namespace views