Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1865063004:
views: support vertical-drag-to-end on textfields (Closed)
|
Created:
4 years, 8 months ago by Elly Fong-Jones Modified:
4 years, 8 months ago CC:
chromium-reviews, tfarina, James Su, shuchen+watch_chromium.org, yusukes+watch_chromium.org, nona+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionviews: support vertical-drag-to-end on textfields
On Mac, dragging above the top of a textfield drags to the beginning of the
textfield's contents, and dragging below the bottom drags to the end of the
textfield's contents. This change implements that behavior, controlled by
PlatformStyle::kTextfieldDragVerticallyDragsToEnd, which is true only on Mac.
BUG=600166
Committed: https://crrev.com/ef378f2dcd244781ba68cc2a4f709e6f3d100569
Cr-Commit-Position: refs/heads/master@{#387988}
Patch Set 1 #
Total comments: 6
Patch Set 2 : move logic into Textfield #
Total comments: 6
Patch Set 3 : Document unit test! #
Total comments: 4
Patch Set 4 : Refactor some textfield unittests #
Total comments: 21
Patch Set 5 : Test fixups for new API #
Total comments: 1
Patch Set 6 : fix test on win/linux #
Messages
Total messages: 30 (9 generated)
Description was changed from ========== views: support vertical-drag-to-end on textfields On Mac, dragging above the top of a textfield drags to the beginning of the textfield's contents, and dragging below the bottom drags to the end of the textfield's contents. This change implements that behavior, controlled by PlatformStyle::kTextfieldDragVerticallyDragsToEnd, which is true only on Mac. BUG=600166 ========== to ========== views: support vertical-drag-to-end on textfields On Mac, dragging above the top of a textfield drags to the beginning of the textfield's contents, and dragging below the bottom drags to the end of the textfield's contents. This change implements that behavior, controlled by PlatformStyle::kTextfieldDragVerticallyDragsToEnd, which is true only on Mac. BUG=600166 ==========
ellyjones@chromium.org changed reviewers: + tapted@chromium.org
tapted: ptal? :)
https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... File ui/views/controls/textfield/textfield_model.cc (right): https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... ui/views/controls/textfield/textfield_model.cc:404: void TextfieldModel::MoveCursorToStart(bool select) { I think these are misleading "Start" could be left in LTR but right in RTL, but in fact we always want geometric left or right, since it's based on the mouse cursor position. Perhaps just one method: SelectLineFromCursor(gfx::VisualCursorDirection direction); but I also have to ask whether we really need the separate method. It changes the testing scene, but - the added tests are mostly redundant with the coverage in TextfieldModelTest.Selection, and - we really should have a test at a higher level with platform-specifics in textfield_unittest.cc that passes a mouse location Perhaps we can just add a few lines to TextfieldTest.DragToSelect (it has a drag_left/right, we can add a drag_up/down as well :). Then, I think it's best for Textfield::SelectThroughLastDragLocation() to just call MoveCursor(..) directly https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... File ui/views/controls/textfield/textfield_model.h (right): https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... ui/views/controls/textfield/textfield_model.h:127: void MoveCursorToStart(bool select); nit `select` is always in this code, so in Chrome's don't-add-it-till-you-need-it methodology, it shouldn't be passed in yet https://codereview.chromium.org/1865063004/diff/1/ui/views/style/platform_sty... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/1865063004/diff/1/ui/views/style/platform_sty... ui/views/style/platform_style.h:42: static const bool kTextfieldDragVerticallyDragsToEnd; this should be ordered before all the functions (see e.g. http://crrev.com/1569113002/diff/470001/ui/views/style/platform_style.h where we will git-conflict :p). order should be adjusted in the .cc's too
tapted: ptal? https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... File ui/views/controls/textfield/textfield_model.cc (right): https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... ui/views/controls/textfield/textfield_model.cc:404: void TextfieldModel::MoveCursorToStart(bool select) { On 2016/04/06 23:29:38, tapted wrote: > I think these are misleading "Start" could be left in LTR but right in RTL, but > in fact we always want geometric left or right, since it's based on the mouse > cursor position. > > Perhaps just one method: > > SelectLineFromCursor(gfx::VisualCursorDirection direction); > > but I also have to ask whether we really need the separate method. It changes > the testing scene, but > - the added tests are mostly redundant with the coverage in > TextfieldModelTest.Selection, and > - we really should have a test at a higher level with platform-specifics in > textfield_unittest.cc that passes a mouse location > > Perhaps we can just add a few lines to TextfieldTest.DragToSelect > > (it has a drag_left/right, we can add a drag_up/down as well :). > > > Then, I think it's best for Textfield::SelectThroughLastDragLocation() to just > call MoveCursor(..) directly Okay, done and removed, and I added a unit test. https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... File ui/views/controls/textfield/textfield_model.h (right): https://codereview.chromium.org/1865063004/diff/1/ui/views/controls/textfield... ui/views/controls/textfield/textfield_model.h:127: void MoveCursorToStart(bool select); On 2016/04/06 23:29:38, tapted wrote: > nit `select` is always in this code, so in Chrome's > don't-add-it-till-you-need-it methodology, it shouldn't be passed in yet Removed these methods. https://codereview.chromium.org/1865063004/diff/1/ui/views/style/platform_sty... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/1865063004/diff/1/ui/views/style/platform_sty... ui/views/style/platform_style.h:42: static const bool kTextfieldDragVerticallyDragsToEnd; On 2016/04/06 23:29:38, tapted wrote: > this should be ordered before all the functions (see e.g. > http://crrev.com/1569113002/diff/470001/ui/views/style/platform_style.h where we > will git-conflict :p). order should be adjusted in the .cc's too Done.
https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... File ui/views/controls/textfield/textfield_unittest.cc (right): https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1199: TEST_F(TextfieldTest, DragUpOrDownSelectsToEnd) { nit: although the other tests are missing one,... it's usually good to have a comment explaining the purpose of a test just above https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1200: if (!PlatformStyle::kTextfieldDragVerticallyDragsToEnd) Can we verify the behaviour when this is false as well? (i.e. to verify that it's *not* selecting more text ?) https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1208: ui::MouseEvent click_start(ui::ET_MOUSE_PRESSED, start_point, start_point, Can these be simplified with a ui::EventGenerator? E.g. they have DragMouseTo(dx, dy) methods to encapsulate a lot of this I meant to toy with this today, but I ran out of time. I think it will make this test a lot more readable. (the other tests in this file should probably use it too, but EventGenerator came after them)
tapted: ptal? :) https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... File ui/views/controls/textfield/textfield_unittest.cc (right): https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1199: TEST_F(TextfieldTest, DragUpOrDownSelectsToEnd) { On 2016/04/08 08:53:28, tapted wrote: > nit: although the other tests are missing one,... it's usually good to have a > comment explaining the purpose of a test just above Done. https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1200: if (!PlatformStyle::kTextfieldDragVerticallyDragsToEnd) On 2016/04/08 08:53:28, tapted wrote: > Can we verify the behaviour when this is false as well? (i.e. to verify that > it's *not* selecting more text ?) Yes. I changed the test to test the correct behavior either way, and tested it both ways on Mac. https://codereview.chromium.org/1865063004/diff/20001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1208: ui::MouseEvent click_start(ui::ET_MOUSE_PRESSED, start_point, start_point, On 2016/04/08 08:53:28, tapted wrote: > Can these be simplified with a ui::EventGenerator? > > E.g. they have DragMouseTo(dx, dy) methods to encapsulate a lot of this > > I meant to toy with this today, but I ran out of time. I think it will make this > test a lot more readable. > > (the other tests in this file should probably use it too, but EventGenerator > came after them) I spent a bit of time trying to use EventGenerator, and I can't figure out how to use it, or debug why it's not working. The docs for the constructor say: // Creates an EventGenerator with the mouse/touch location centered over // |window|. This is currently the only constructor that works on Mac, since // a specific window is required (and there is no root window). EventGenerator(gfx::NativeWindow root_window, gfx::NativeWindow window); But I think that since this is a Views build, there is no native window for the actual textfield control. I tried mirroring the pattern of other uses of EventGenerator in the views controls unit tests, but they seem to do this: EventGenerator(GetContext(), widget_->GetNativeWindow()) I tried doing this, and no events are delivered to my widget, no matter whether I use screen coordinates or widget-local coordinates for the events. I looked at the EventGenerator code but it is very complex and I found it pretty much impossible to debug why my events were not being delivered. Further investigation revealed that GetContext() returns NULL on non-Aura builds, so that seems like it should be broken, but it works in other places, so I'm a bit baffled by that. Anyway, I guess the short version of how I feel is that if I can't figure out how to use EventGenerator or why it isn't working, it's likely that future people who have to maintain this code won't be able to figure it out either, so I shouldn't use it here. The current code is kind of verbose, but it calls event handlers directly on the widget and the code is very easy to follow. If you feel strongly that I should use EventGenerator, I can spend more time trying to understand it, but otherwise I'd prefer to leave this as it is.
There's already an event_generator_ member on TextfieldTest - here's how the test would look to use it https://codereview.chromium.org/1875003002/diff2/1:20001/ui/views/controls/te... However, there were a couple of bugs. one in textfield_unittest.cc (in that diff), and another more subtle one in the Mac event generator delegate: https://codereview.chromium.org/1875003002/diff2/1:20001/ui/views/test/event_... I've started some jobs in https://codereview.chromium.org/1875003002/ - it's possible fixing the mac event generator delegate causes problems elsewhere. https://codereview.chromium.org/1865063004/diff/40001/ui/views/controls/textf... File ui/views/controls/textfield/textfield.cc (right): https://codereview.chromium.org/1865063004/diff/40001/ui/views/controls/textf... ui/views/controls/textfield/textfield.cc:1755: true); nit: git cl format puts this `true` on the previous line, same below https://codereview.chromium.org/1865063004/diff/40001/ui/views/style/platform... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/1865063004/diff/40001/ui/views/style/platform... ui/views/style/platform_style.h:25: static const bool kTextfieldDragVerticallyDragsToEnd; (the button style CL landed, so you'll need to rebase for a conflict here)
> I've started some jobs inhttps <https://codereview.chromium.org/1875003002/>:// <https://codereview.chromium.org/1875003002/>codereview.chromium.org <https://codereview.chromium.org/1875003002/>/1875003002/ <https://codereview.chromium.org/1875003002/> - it's possible fixing the mac event generator delegate causes problems elsewhere. So no surprise regressions on Mac (didn't check the rest of views_unittests though). But failures on aura. I think the mouse coordinates are relative to the root window there -- not the widget. Widget origin is (100,100), so the need to be offset. Mouse location starts in the middle of the window -- we could rely on that and use relative move/drag methods. Or do something like EventGenerator::SetMouseRelativeToWindow(..). (Which doesn't exist -- we'd have to add it). But I think we need something to make EventGenerator less painful to use for these kind of tests going forward. -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 12 Apr 2016 22:44, "Trent Apted" <tapted@google.com> wrote: > > > I've started some jobs inhttps://codereview.chromium.org/1875003002/ - it's > > possible fixing the mac event generator delegate causes problems elsewhere. > > So no surprise regressions on Mac (didn't check the rest of views_unittests though). But failures on aura. I think the mouse coordinates are relative to the root window there -- not the widget. Widget origin is (100,100), so the need to be offset. > > Mouse location starts in the middle of the window -- we could rely on that and use relative move/drag methods. Or do something like EventGenerator::SetMouseRelativeToWindow(..). (Which doesn't exist -- we'd have to add it). > > But I think we need something to make EventGenerator less painful to use for these kind of tests going forward. Ooh - maybe we just need to update the Mac event generator delegate to offset coordinates by its window. I.e. make it pretend it's in a root window as well. -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/04/12 13:00:57, chromium-reviews wrote: > On 12 Apr 2016 22:44, "Trent Apted" <mailto:tapted@google.com> wrote: > > > > > I've started some jobs inhttps://codereview.chromium.org/1875003002/ - > it's > > > > possible fixing the mac event generator delegate causes problems > elsewhere. > > > > So no surprise regressions on Mac (didn't check the rest of > views_unittests though). But failures on aura. I think the mouse > coordinates are relative to the root window there -- not the widget. Widget > origin is (100,100), so the need to be offset. > > > > Mouse location starts in the middle of the window -- we could rely on > that and use relative move/drag methods. Or do something like > EventGenerator::SetMouseRelativeToWindow(..). (Which doesn't exist -- we'd > have to add it). > > > > But I think we need something to make EventGenerator less painful to use > for these kind of tests going forward. > > Ooh - maybe we just need to update the Mac event generator delegate to > offset coordinates by its window. I.e. make it pretend it's in a root > window as well. > > -- > You received this message because you are subscribed to the Google Groups > "Chromium-reviews" group. > To unsubscribe from this group and stop receiving emails from it, send an email > to mailto:chromium-reviews+unsubscribe@chromium.org. I don't really want to use EventGenerator for this CL. Is it okay with you if I land it without using EventGenerator?
I wrote some wrappers in TextfieldTest that can be converted to use EventGenerator later (they match its API). What do you think? https://codereview.chromium.org/1865063004/diff/40001/ui/views/controls/textf... File ui/views/controls/textfield/textfield.cc (right): https://codereview.chromium.org/1865063004/diff/40001/ui/views/controls/textf... ui/views/controls/textfield/textfield.cc:1755: true); On 2016/04/11 06:57:26, tapted wrote: > nit: git cl format puts this `true` on the previous line, same below Done. https://codereview.chromium.org/1865063004/diff/40001/ui/views/style/platform... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/1865063004/diff/40001/ui/views/style/platform... ui/views/style/platform_style.h:25: static const bool kTextfieldDragVerticallyDragsToEnd; On 2016/04/11 06:57:26, tapted wrote: > (the button style CL landed, so you'll need to rebase for a conflict here) Done.
nice - I like this approach. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... File ui/views/controls/textfield/textfield_unittest.cc (right): https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1126: // ui::MouseEvent click(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), (delete commented stuff) https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1183: ClickLeftMouseButton(); This needs to pass in a parameter to do a double-click. Perhaps an `int extra_flags`, and pass ui::EF_IS_DOUBLE_CLICK here? https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1344: // ui::MouseEvent press_event(ui::ET_MOUSE_PRESSED, kStringPoint, (remove) https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1377: // ui::MouseEvent click_a(ui::ET_MOUSE_PRESSED, point, point, (remove) https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1381: EXPECT_TRUE(textfield_->CanStartDragForView(textfield_, point, gfx::Point())); The third argument for CanStartDragForView is the current mouse coordinate, so that should probably be |point| as well. But I guess views::TextField ignores it either way, so I'm indifferent https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1385: textfield_->WriteDragDataForView(NULL, point, &data); nit: nullptr https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1431: // ui::MouseEvent click_a(ui::ET_MOUSE_PRESSED, point, point, (remove) https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1439: textfield_->WriteDragDataForView(NULL, point, &data); nit: nullptr https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1480: textfield_->WriteDragDataForView(NULL, point, &data); nit: nullptr https://codereview.chromium.org/1865063004/diff/60001/ui/views/style/platform... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/1865063004/diff/60001/ui/views/style/platform... ui/views/style/platform_style.h:23: VIEWS_EXPORT static const bool kTextfieldDragVerticallyDragsToEnd; #include ui/views/views_export.h for this. Also I think we should just VIEWS_EXPORT the entire class - pretty sure the static member will inherit that (and there's not point being stingy with exports since it #defines to the empty string in release builds)
ptal? :) https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... File ui/views/controls/textfield/textfield_unittest.cc (right): https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1126: // ui::MouseEvent click(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), On 2016/04/15 00:35:08, tapted wrote: > (delete commented stuff) Done. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1183: ClickLeftMouseButton(); On 2016/04/15 00:35:08, tapted wrote: > This needs to pass in a parameter to do a double-click. Perhaps an `int > extra_flags`, and pass ui::EF_IS_DOUBLE_CLICK here? I added overloaded versions of ClickLeftMouseButton and PressLeftMouseButton that take extra flags, and made this call site use them. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1344: // ui::MouseEvent press_event(ui::ET_MOUSE_PRESSED, kStringPoint, On 2016/04/15 00:35:08, tapted wrote: > (remove) Done. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1377: // ui::MouseEvent click_a(ui::ET_MOUSE_PRESSED, point, point, On 2016/04/15 00:35:08, tapted wrote: > (remove) Done. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1381: EXPECT_TRUE(textfield_->CanStartDragForView(textfield_, point, gfx::Point())); On 2016/04/15 00:35:08, tapted wrote: > The third argument for CanStartDragForView is the current mouse coordinate, so > that should probably be |point| as well. But I guess views::TextField ignores it > either way, so I'm indifferent point is more correct so point it is. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1385: textfield_->WriteDragDataForView(NULL, point, &data); On 2016/04/15 00:35:08, tapted wrote: > nit: nullptr Done. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1431: // ui::MouseEvent click_a(ui::ET_MOUSE_PRESSED, point, point, On 2016/04/15 00:35:08, tapted wrote: > (remove) Done. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1439: textfield_->WriteDragDataForView(NULL, point, &data); On 2016/04/15 00:35:08, tapted wrote: > nit: nullptr Done. https://codereview.chromium.org/1865063004/diff/60001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:1480: textfield_->WriteDragDataForView(NULL, point, &data); On 2016/04/15 00:35:08, tapted wrote: > nit: nullptr Done. https://codereview.chromium.org/1865063004/diff/60001/ui/views/style/platform... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/1865063004/diff/60001/ui/views/style/platform... ui/views/style/platform_style.h:23: VIEWS_EXPORT static const bool kTextfieldDragVerticallyDragsToEnd; On 2016/04/15 00:35:08, tapted wrote: > #include ui/views/views_export.h for this. Also I think we should just > VIEWS_EXPORT the entire class - pretty sure the static member will inherit that > (and there's not point being stingy with exports since it #defines to the empty > string in release builds) After a rebase, it seems like it is already included, and the entire class is already VIEWS_EXPORTed?
lgtm https://codereview.chromium.org/1865063004/diff/80001/ui/views/controls/textf... File ui/views/controls/textfield/textfield_unittest.cc (right): https://codereview.chromium.org/1865063004/diff/80001/ui/views/controls/textf... ui/views/controls/textfield/textfield_unittest.cc:645: void PressLeftMouseButton() { (note style guide now allows default arguments if you prefer that. Although I'm somewhat dubious of default arguments for things like `int` which don't carry a lot of intrinsic meaning - e.g. it's easy to add a second `int` arg and get lost. So overloading is fine. I'd also be fine with no overload and passing `0` to everything explicitly.)
https://codereview.chromium.org/1865063004/diff/60001/ui/views/style/platform... File ui/views/style/platform_style.h (right): https://codereview.chromium.org/1865063004/diff/60001/ui/views/style/platform... ui/views/style/platform_style.h:23: VIEWS_EXPORT static const bool kTextfieldDragVerticallyDragsToEnd; On 2016/04/15 14:47:46, Elly Jones wrote: > On 2016/04/15 00:35:08, tapted wrote: > > #include ui/views/views_export.h for this. Also I think we should just > > VIEWS_EXPORT the entire class - pretty sure the static member will inherit > that > > (and there's not point being stingy with exports since it #defines to the > empty > > string in release builds) > > After a rebase, it seems like it is already included, and the entire class is > already VIEWS_EXPORTed? ahyep - that was me :3
ellyjones@chromium.org changed reviewers: + sky@chromium.org
sky: ptal? :)
LGTM
The CQ bit was checked by ellyjones@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1865063004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1865063004/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by ellyjones@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org, sky@chromium.org Link to the patchset: https://codereview.chromium.org/1865063004/#ps100001 (title: "fix test on win/linux")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1865063004/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1865063004/100001
Message was sent while issue was closed.
Description was changed from ========== views: support vertical-drag-to-end on textfields On Mac, dragging above the top of a textfield drags to the beginning of the textfield's contents, and dragging below the bottom drags to the end of the textfield's contents. This change implements that behavior, controlled by PlatformStyle::kTextfieldDragVerticallyDragsToEnd, which is true only on Mac. BUG=600166 ========== to ========== views: support vertical-drag-to-end on textfields On Mac, dragging above the top of a textfield drags to the beginning of the textfield's contents, and dragging below the bottom drags to the end of the textfield's contents. This change implements that behavior, controlled by PlatformStyle::kTextfieldDragVerticallyDragsToEnd, which is true only on Mac. BUG=600166 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== views: support vertical-drag-to-end on textfields On Mac, dragging above the top of a textfield drags to the beginning of the textfield's contents, and dragging below the bottom drags to the end of the textfield's contents. This change implements that behavior, controlled by PlatformStyle::kTextfieldDragVerticallyDragsToEnd, which is true only on Mac. BUG=600166 ========== to ========== views: support vertical-drag-to-end on textfields On Mac, dragging above the top of a textfield drags to the beginning of the textfield's contents, and dragging below the bottom drags to the end of the textfield's contents. This change implements that behavior, controlled by PlatformStyle::kTextfieldDragVerticallyDragsToEnd, which is true only on Mac. BUG=600166 Committed: https://crrev.com/ef378f2dcd244781ba68cc2a4f709e6f3d100569 Cr-Commit-Position: refs/heads/master@{#387988} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/ef378f2dcd244781ba68cc2a4f709e6f3d100569 Cr-Commit-Position: refs/heads/master@{#387988} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
