New issue
Advanced search Search tips

Issue 708401 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug
M-X

Blocking:
issue 672343



Sign in to add a comment

Fix views_unittests relying on EventGenerator::assume_window_at_origin()

Project Member Reported by tapted@chromium.org, Apr 5 2017

Issue description

Chrome Version       : 59.0.3047.0
OS Version: OS X 10.12.3

A bunch of tests using EventGenerator assume the Widget that they show at (0,0) is at (0,0) after showing and so don't bother with coordinate conversions.

On Mac, the WindowServer may move the window to a different place so that it doesn't overlap the mainMenu bar.

https://codereview.chromium.org/2794213002/ adds EventGenerator::assume_window_at_origin() with the comment:

  // Many tests assume a window created at (0,0) will remain there when shown.
  // However, an operating system's window manager may reposition the window
  // into the work area. This can disrupt the coordinates used on test events,
  // so an EventGeneratorDelegate may skip the step that remaps coordinates in
  // the root window to window coordinates when dispatching events.
  // Setting this to false skips that step, in which case the test must ensure
  // it correctly maps coordinates in window coordinates to root window (screen)
  // coordinates when calling, e.g., set_current_location().
  // Default is true. This only has any effect on Mac.
  void set_assume_window_at_origin(bool assume_window_at_origin)


The failing views_unittests are:

CustomButtonTest.HideInkDropHighlightOnDisable
CustomButtonTest.HoverStateOnVisibilityChange
InkDropLabelButtonTest.HoverStateAfterMouseEnterAndExitEvents
MenuButtonTest.ActivateDropDownOnMouseClick
MenuButtonTest.ButtonStateForMenuButtonsWithPressedLocks
MenuButtonTest.DraggableMenuButtonActivatesOnRelease
MenuButtonTest.PressedStateWithSiblingMenu
MenuRunnerWidgetTest.WidgetDoesntTakeCapture
NativeWidgetMacTest.Tooltips
NativeWidgetMacTest.TwoWidgetTooltips
SliderTest.UpdateFromClickHorizontal
SliderTest.UpdateFromClickRTLHorizontal
WidgetTest.CaptureDuringMousePressNotOverridden
WidgetTest.MouseEventTypesViaGenerator
WidgetTest.MousePressCausesCapture
WidgetTest.SynthesizeMouseMoveEvent

and one interactive_ui_test:

WidgetCaptureTest.FailedCaptureRequestIsNoop



E.g. there's code like


TEST_F(CustomButtonTest, HideInkDropHighlightOnDisable) {
  TestInkDrop* ink_drop = new TestInkDrop();
  CreateButtonWithInkDrop(base::WrapUnique(ink_drop), false);

  ui::test::EventGenerator generator(widget()->GetNativeWindow());
  generator.MoveMouseToInHost(10, 10);


A nice fix might be to add a methods like MoveMouseToInWindow, so this becomes

  ui::test::EventGenerator generator(widget()->GetNativeWindow());
  generator.MoveMouseToInWindow(widget()->GetNativeWindow(), 10, 10);


Where MoveMouseToInWindow moves points relative to the content area of the provided gfx::NativeWindow (i.e. ignores titlebar as well).
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1746a908f94b40aea3c37d63734d4667e3835bc

commit a1746a908f94b40aea3c37d63734d4667e3835bc
Author: tapted <tapted@chromium.org>
Date: Thu Apr 06 03:19:31 2017

MacViews: Fix some TableView tests, add ui::EventGenerator::set_assume_window_at_origin(bool)

EventGenerator uses root window coordinates, and many tests using
EventGenerator assume that a window it creates at (0,0) will be placed
at the origin of the root window when Show()n. However, on Mac, this is
rarely the case for "real" Widgets. The WindowServer forcefully shifts
the window into the work area so that it doesn't overlap the OS mainMenu
bar.

On Mac, the EventGenerator currently doesn't remap mouse coordinates to
a "hit" window: effectively treating all windows as if they are
positioned at (0,0). But tests that *do* correctly take into account the
screen position of a test window, and correctly map their test event
coordinates, will not like this.

By calling EventGenerator::set_assume_window_at_origin(false) a test can
disable the "fake" coordinate mapping.

Using EventGenerator and "real" NSEvents exposed some other bugs in
TreeView. Ctrl+LeftClick on Mac is actually a right-click so can't be
used for multi-row selection. Update TableView and its tests to use
Command on Mac.

Also the Mac EventGenerator was passing event->changed_button_flags() as
the event modifiers, but it should pass event->flags() instead for mouse
events. (Only NSFlagsChanged key events should use
changed_button_flags(), but EventGenerator hasn't needed them yet.

BUG=708401

Review-Url: https://codereview.chromium.org/2794213002
Cr-Commit-Position: refs/heads/master@{#462342}

[modify] https://crrev.com/a1746a908f94b40aea3c37d63734d4667e3835bc/ui/events/test/event_generator.cc
[modify] https://crrev.com/a1746a908f94b40aea3c37d63734d4667e3835bc/ui/events/test/event_generator.h
[modify] https://crrev.com/a1746a908f94b40aea3c37d63734d4667e3835bc/ui/views/controls/table/table_view.cc
[modify] https://crrev.com/a1746a908f94b40aea3c37d63734d4667e3835bc/ui/views/controls/table/table_view_unittest.cc
[modify] https://crrev.com/a1746a908f94b40aea3c37d63734d4667e3835bc/ui/views/test/event_generator_delegate_mac.mm

Labels: -Pri-3 MacViews-Cleanup Pri-2
Labels: M-X
Labels: Group-Tests
Labels: Hotlist-DesktopUIChecked Hotlist-DesktopUIToolingRequired

Sign in to add a comment