New issue
Advanced search Search tips

Issue 592890 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug

Blocking:
issue 500806
issue 579380



Sign in to add a comment

MacViews: new views_unitests failing: SelectChildButtonView, NonLatinMnemonic, HandleAccelerator

Project Member Reported by tapted@chromium.org, Mar 8 2016

Issue description

Chrome Version       : 50.0.2652.0
OS Version: OS X 10.11.3

3 new views_unittests are failing and are not accounted for in  Issue 592856  or http://crrev.com/1772153002 (or caused by http://crrev.com/377477)

- MenuControllerTest.SelectChildButtonView (menu_controller_unittest.cc:723)
- MenuRunnerTest.NonLatinMnemonic (menu_runner_unittest.cc:193)
- ViewTest.HandleAccelerator (view_unittest.cc:2108)

These are all recently-added tests, so they're not regressions - just things that never worked on MacViews.


MenuRunnerTest.NonLatinMnemonic (menu_runner_unittest.cc:193)

Added 2016-02-02 in r372987 -> https://codereview.chromium.org/1652133003

[ RUN      ] MenuRunnerTest.NonLatinMnemonic
../../ui/views/controls/menu/menu_runner_unittest.cc:204: Failure
Value of: runner->IsRunning()
  Actual: true
Expected: false
../../ui/views/controls/menu/menu_runner_unittest.cc:206: Failure
Value of: delegate->execute_command_id()
  Actual: 0
Expected: 2
../../ui/views/controls/menu/menu_runner_unittest.cc:207: Failure
Value of: delegate->on_menu_closed_called()
  Actual: 0
Expected: 1
../../ui/views/controls/menu/menu_runner_unittest.cc:208: Failure
Expected: (nullptr) != (delegate->on_menu_closed_menu()), actual: 8-byte object <00-00 00-00 00-00 00-00> vs NULL
../../ui/views/controls/menu/menu_runner_unittest.cc:209: Failure
Value of: delegate->on_menu_closed_run_result()
  Actual: 0
Expected: MenuRunner::NORMAL_EXIT
Which is: 1
[  FAILED  ] MenuRunnerTest.NonLatinMnemonic (84 ms)


ViewTest.HandleAccelerator (view_unittest.cc:2108)

Added 2016-02-10 in r374787 -> https://codereview.chromium.org/1544803004

[ RUN      ] ViewTest.HandleAccelerator
../../ui/views/view_unittest.cc:2138: Failure
Value of: widget->IsActive()
  Actual: false
Expected: true
[  FAILED  ] ViewTest.HandleAccelerator (96 ms)


MenuControllerTest.SelectChildButtonView (menu_controller_unittest.cc:723)

Added 2016-02-29 in r378300 -> https://codereview.chromium.org/1741093002

[ RUN      ] MenuControllerTest.SelectChildButtonView
../../ui/views/controls/menu/menu_controller_unittest.cc:776: Failure
Value of: pending_state_item()->GetCommand()
  Actual: 1
Expected: 5
../../ui/views/controls/menu/menu_controller_unittest.cc:778: Failure
Value of: button2->IsHotTracked()
  Actual: false
Expected: true
../../ui/views/controls/menu/menu_controller_unittest.cc:784: Failure
Value of: pending_state_item()->GetCommand()
  Actual: 3
Expected: 1
[  FAILED  ] MenuControllerTest.SelectChildButtonView (152 ms)
 

Comment 1 by tapted@chromium.org, Mar 10 2016

Owner: tapted@chromium.org
Status: Started (was: Available)
fix for ViewTest.HandleAccelerator -> https://codereview.chromium.org/1782773002

Comment 2 by tapted@chromium.org, Mar 10 2016

MenuRunnerTest.NonLatinMnemonic -> https://codereview.chromium.org/1779743004
MenuControllerTest.SelectChildButtonView -> https://codereview.chromium.org/1778243003
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 10 2016

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

commit e8acb685ae8b680ca50b611ce5cd0dc0fc2506c6
Author: tapted <tapted@chromium.org>
Date: Thu Mar 10 06:29:25 2016

MacViews: Fix MenuRunnerTest.NonLatinMnemonic

The test NSEvent synthesizer didn't handle unicode characters. Add
support for this by feeding through the DomKey from "character"
ui::KeyEvents.

BUG= 592890 

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

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

[modify] https://crrev.com/e8acb685ae8b680ca50b611ce5cd0dc0fc2506c6/ui/events/test/cocoa_test_event_utils.h
[modify] https://crrev.com/e8acb685ae8b680ca50b611ce5cd0dc0fc2506c6/ui/events/test/cocoa_test_event_utils.mm
[modify] https://crrev.com/e8acb685ae8b680ca50b611ce5cd0dc0fc2506c6/ui/views/test/event_generator_delegate_mac.mm

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 11 2016

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

commit bb2a58ae2441fbfa4f5168baab1b1a5c0c047916
Author: tapted <tapted@chromium.org>
Date: Fri Mar 11 00:22:33 2016

MacViews: Fix MenuControllerTest.SelectChildButtonView

Above the declaration for MenuController::OnMouseMoved(..) it says
  // NOTE: the coordinates of the events are in that of the
  // MenuScrollViewContainer.
But the test wasn't doing that.

I think on Mac the menu Widget is shifted down by the height of the
main menu bar, so the test couldn't assume it's at the top left of
the screen.

BUG= 592890 

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

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

[modify] https://crrev.com/bb2a58ae2441fbfa4f5168baab1b1a5c0c047916/ui/views/controls/menu/menu_controller_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 14 2016

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

commit 2a697015fafd0c99b169890a9a7ac0b2011a91c0
Author: tapted <tapted@chromium.org>
Date: Mon Mar 14 02:00:42 2016

MacViews: Fix ViewTest.HandleAccelerator by faking window activation

In fact, fake activation for all tests that use ViewsTestHelper, except
those that run as interactive_ui_tests.

Currently for HandleAccelerator, `EXPECT_TRUE(widget->IsActive())` fails
on Mac since desktop widgets are used rather than Aura widgets, so
activation is asynchronous.

Also TYPE_POPUP widgets are not activatable by default (on all
platforms). For non-desktop Aura widgets, Show() on non-activatable
widgets does not activate, but Activate() does allow activation.

Make it easier for a unit test inheriting from ViewsTestBase to use fake
window activation on Mac without using #ifdefs by just giving it to them
by default.

BUG= 592890 

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

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

[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/base/test/ui_controls.h
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/base/test/ui_controls_mac.mm
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/BUILD.gn
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/accessible_pane_view_unittest.cc
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/bubble/bubble_delegate_unittest.cc
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/cocoa/bridged_native_widget_interactive_uitest.mm
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/controls/textfield/textfield_unittest.cc
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/test/views_test_helper_mac.h
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/test/views_test_helper_mac.mm
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/test/widget_test.h
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/test/widget_test_mac.mm
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/view_unittest.cc
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/views.gyp
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/widget/native_widget_mac_interactive_uitest.mm
[modify] https://crrev.com/2a697015fafd0c99b169890a9a7ac0b2011a91c0/ui/views/window/dialog_delegate_unittest.cc

Comment 6 by tapted@chromium.org, Mar 14 2016

Status: Fixed (was: Started)
Labels: -Hotlist-MacViews Proj-MacViews

Sign in to add a comment