New issue
Advanced search Search tips

Issue 730886 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

mash: App List context menu mouse interaction broken.

Project Member Reported by msw@chromium.org, Jun 8 2017

Issue description

mash: "Pin To Shelf" in AppList context menus doesn't actually pin items.

In chrome --mash:
1) Open the app list.
2) Trigger an unpinned item's context menu (eg. Files app).
3) Select "Ping to Shelf"
Expected: pinned shelf item created.
Actual: no pinned shelf item created.

This is related to (blocking)  Issue 557406 , and (blocking?)  Issue 681072 .
 

Comment 1 by msw@chromium.org, Aug 24 2017

Cc: e...@chromium.org riajiang@chromium.org
Components: Internals>WindowsServer
Summary: mash: App List context menu mouse press dispatch broken. (was: mash: "Pin To Shelf" in AppList context menus doesn't actually pin items.)
The app list context menu items work just fine if invoked with a key event.
None work when invoked with a mouse event; this is an event dispatch bug.

Oddly, the correct menu item is highlighted on hover, only mouse press is broken.
Also, clicking the app list window while a menu is open closes the menu & window.

I'm debugging services/ui/ws/event_dispatcher.cc and window_manager_state.cc
Any tips or advice here is appreciated, I'm not very familiar with this new code.

Comment 2 by msw@chromium.org, Aug 24 2017

Status: Started (was: Assigned)
Summary: mash: App List context menu mouse interaction broken. (was: mash: App List context menu mouse press dispatch broken.)
Menu item selection occurs on mouse up, but the menu and app list close on mouse down.
So it's not quite an event dispatch issue, I'm looking into the cause of window close.

Comment 3 by msw@chromium.org, Aug 24 2017

Blocking: -681072 -557406
This isn't shelf related, removing blocker status, but I'll keep looking anyway.

Comment 4 by msw@chromium.org, Aug 24 2017

Cc: -e...@chromium.org -riajiang@chromium.org
Oh gosh, AppListPresenterDelegateMus::OnPointerEventObserved dismisses the app list for touch/pointer down events on widgets other than the app list, including context menus that it's showing... I'm working on a fix. It was difficult to trace back the widget closing through the WindowTree[Client] and FocusSynchonizer hops, but I'm pretty sure this is the root cause.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 26 2017

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

commit a85ad6142b28e405a5ee7d6e6f86b8980115c1a0
Author: Mike Wasserman <msw@chromium.org>
Date: Sat Aug 26 00:26:15 2017

mash: Do not dismiss the app list for events on its context menus.

Check for the context menu window as a PointerWatcher target.
(this mash-specific fix checks a complex transient hierarchy)
Add a browser test (TODO: fix shutdown crash in --run-in-mash)

Bug:  730886 
Change-Id: I6d16b98b1394cb3e57d176c7be2b3ea5d83cc0fe
Reviewed-on: https://chromium-review.googlesource.com/636183
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497609}
[add] https://crrev.com/a85ad6142b28e405a5ee7d6e6f86b8980115c1a0/chrome/browser/ui/ash/app_list/app_list_browsertest.cc
[delete] https://crrev.com/c1b4adf9c2f60c8dc45bfebf8ba92b319f912b5b/chrome/browser/ui/ash/app_list/app_list_controller_ash_browsertest.cc
[modify] https://crrev.com/a85ad6142b28e405a5ee7d6e6f86b8980115c1a0/chrome/browser/ui/ash/app_list/app_list_presenter_delegate_mus.cc
[modify] https://crrev.com/a85ad6142b28e405a5ee7d6e6f86b8980115c1a0/chrome/test/BUILD.gn

Comment 6 by msw@chromium.org, Aug 26 2017

Status: Fixed (was: Started)

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment