New issue
Advanced search Search tips

Issue 595853 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 557406



Sign in to add a comment

Mash Shell Shelf Tooltips

Project Member Reported by msw@chromium.org, Mar 17 2016

Issue description

Mash Shell Shelf Tooltips

ShelfTooltipManager doesn't show bubble tooltips for shelf items in mash.

See parent  Issue 557406 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 23 2016

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

commit e9d901efb5bc6bbf434ac67b70f90e9ef73419d8
Author: msw <msw@chromium.org>
Date: Wed Mar 23 23:48:50 2016

Enable mash shelf tooltips.

Make ShelfItemDelegateMus::ShouldShowTooltip() return true.
Put sysui menu/bubble widgets in mus's MENUS container.
(bubbles are used for shelf tooltips and volume/etc. settings)
Put sysui tooltip widgets in mus's TOOLTIPS container.

Rewrite ShelfTooltipManager as an aura::Window pre-target handler.
(can't install itself as a pre-target handler for the desktop root...)
Remove ShelfButtonHost; simplify ShelfButton and AppListButton.
(contain event observation logic within ShelfTooltipManager)

Update unit tests and helper classes; note currently broken behavior:

TODO: Make tooltips close on touch events outside the shelf.
TODO: Push item state to items; avoid shelf item delegates, etc.

BUG= 595853 
TEST=mash shelf shows tooltips; no cros regressions (except not closing on touch/gesture outside shelf).
R=sky@chromium.org

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

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

[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/ash.gyp
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/mus/shelf_delegate_mus.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/mus/sysui_application.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/app_list_button.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/app_list_button.h
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_button.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_button.h
[delete] https://crrev.com/fbaed4750ac46ce84d4b5d361b2601447db5390d/ash/shelf/shelf_button_host.h
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_tooltip_manager.h
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_view.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_view.h
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/test/shelf_test_api.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/test/shelf_view_test_api.cc
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ash/test/shelf_view_test_api.h
[modify] https://crrev.com/e9d901efb5bc6bbf434ac67b70f90e9ef73419d8/ui/app_list/views/app_list_drag_and_drop_host.h

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 25 2016

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

commit 1a0d283fa81e19b47fb85523f28c4a03f7dc1d24
Author: msw <msw@chromium.org>
Date: Fri Mar 25 23:31:12 2016

Refine ash shelf tooltip closing on non-mash ChromeOS.

Fixes a regression from: https://codereview.chromium.org/1816753002
Also reverse expected behavior for touching tooltips.
(closes on stable, tests expected them to stay open...)

Inline bubble function definitions.
Call WillDeleteShelf earlier to fix a test crash...
TODO: Encapsulate more ShelfLayoutManager code.

BUG= 595853 
TEST=ChromeOS tooltips stay open on hover; close for external touches.
R=sky@chromium.org

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

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

[modify] https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24/ash/shelf/shelf_tooltip_manager_unittest.cc

Project Member

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

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

commit 053ef0ef0cb9765594615de430fad25b24d100c6
Author: msw <msw@chromium.org>
Date: Sat Mar 26 04:18:57 2016

Revert of Refine ash shelf tooltip closing on non-mash ChromeOS. (patchset #4 id:60001 of https://codereview.chromium.org/1828133004/ )

Reason for revert:
Memory failures, eg.
https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20ChromeOS%20MSan%20Tests/builds/8340

Original issue's description:
> Refine ash shelf tooltip closing on non-mash ChromeOS.
>
> Fixes a regression from: https://codereview.chromium.org/1816753002
> Also reverse expected behavior for touching tooltips.
> (closes on stable, tests expected them to stay open...)
>
> Inline bubble function definitions.
> Call WillDeleteShelf earlier to fix a test crash...
> TODO: Encapsulate more ShelfLayoutManager code.
>
> BUG= 595853 
> TEST=ChromeOS tooltips stay open on hover; close for external touches.
> R=sky@chromium.org
>
> Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24
> Cr-Commit-Position: refs/heads/master@{#383404}

TBR=sky@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 595853 

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

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

[modify] https://crrev.com/053ef0ef0cb9765594615de430fad25b24d100c6/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/053ef0ef0cb9765594615de430fad25b24d100c6/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/053ef0ef0cb9765594615de430fad25b24d100c6/ash/shelf/shelf_tooltip_manager_unittest.cc

Comment 4 by msw@chromium.org, Mar 28 2016

Owner: msw@chromium.org
Status: Started (was: Available)
Project Member

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

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

commit c2aa7f5e8870c04c8aff0fd5da27820636e53e38
Author: msw <msw@chromium.org>
Date: Mon Mar 28 22:23:07 2016

Refine ash shelf tooltip closing on non-mash ChromeOS.

Fixes a regression from: https://codereview.chromium.org/1816753002
Also reverse expected behavior for touching tooltips.
(closes on stable, tests expected them to stay open...)

Inline bubble function definitions.
Call WillDeleteShelf earlier to fix a test crash...
TODO: Encapsulate more ShelfLayoutManager code.

BUG= 595853 
TEST=ChromeOS tooltips stay open on hover; close for external touches.
R=sky@chromium.org

Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24
Cr-Commit-Position: refs/heads/master@{#383404}

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

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

[modify] https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38/ash/shelf/shelf_widget.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 29 2016

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

commit 327ea5e179d66d730bd6e03486b3494660d84dbc
Author: msw <msw@chromium.org>
Date: Tue Mar 29 00:04:44 2016

Revert of Refine ash shelf tooltip closing on non-mash ChromeOS. (patchset #5 id:80001 of https://codereview.chromium.org/1828133004/ )

Reason for revert:
ash_unittests ShelfLayoutManagerTest.ShutdownHandlesWindowActivation failure on win:
https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/47284

Original issue's description:
> Refine ash shelf tooltip closing on non-mash ChromeOS.
>
> Fixes a regression from: https://codereview.chromium.org/1816753002
> Also reverse expected behavior for touching tooltips.
> (closes on stable, tests expected them to stay open...)
>
> Inline bubble function definitions.
> Call WillDeleteShelf earlier to fix a test crash...
> TODO: Encapsulate more ShelfLayoutManager code.
>
> BUG= 595853 
> TEST=ChromeOS tooltips stay open on hover; close for external touches.
> R=sky@chromium.org
>
> Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24
> Cr-Commit-Position: refs/heads/master@{#383404}
>
> Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38
> Cr-Commit-Position: refs/heads/master@{#383579}

TBR=sky@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 595853 

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

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

[modify] https://crrev.com/327ea5e179d66d730bd6e03486b3494660d84dbc/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/327ea5e179d66d730bd6e03486b3494660d84dbc/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/327ea5e179d66d730bd6e03486b3494660d84dbc/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/327ea5e179d66d730bd6e03486b3494660d84dbc/ash/shelf/shelf_widget.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 29 2016

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

commit 5b1ca37ffa7c373b9b726f786f410bc80348a246
Author: msw <msw@chromium.org>
Date: Tue Mar 29 20:11:01 2016

Refine ash shelf tooltip closing on non-mash ChromeOS.

Fixes a regression from: https://codereview.chromium.org/1816753002
Also reverse expected behavior for touching tooltips.
(closes on stable, tests expected them to stay open...)

Inline bubble function definitions.
Use WindowObserver to remove the handler on window destruction.
TODO: Encapsulate more ShelfLayoutManager code.

BUG= 595853 
TEST=ChromeOS tooltips stay open on hover; close for external touches.
R=sky@chromium.org

Committed: https://crrev.com/1a0d283fa81e19b47fb85523f28c4a03f7dc1d24
Cr-Commit-Position: refs/heads/master@{#383404}

Committed: https://crrev.com/c2aa7f5e8870c04c8aff0fd5da27820636e53e38
Cr-Commit-Position: refs/heads/master@{#383579}

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

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

[modify] https://crrev.com/5b1ca37ffa7c373b9b726f786f410bc80348a246/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/5b1ca37ffa7c373b9b726f786f410bc80348a246/ash/shelf/shelf_tooltip_manager.h
[modify] https://crrev.com/5b1ca37ffa7c373b9b726f786f410bc80348a246/ash/shelf/shelf_tooltip_manager_unittest.cc
[modify] https://crrev.com/5b1ca37ffa7c373b9b726f786f410bc80348a246/ash/shelf/shelf_widget.cc

Project Member

Comment 8 by bugdroid1@chromium.org, May 4 2016

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

commit b53df9e2ff6a08ab6203129e8be41583712099f9
Author: msw <msw@chromium.org>
Date: Wed May 04 21:28:37 2016

Close mash shelf tooltips on touches outside the shelf.

Make ShelfTooltipManager a views::PointerWatcher.
Close on mouse/touch press (including outside shelf root).

BUG= 595853 
TEST=Touches outside the shelf area close shelf tooltips.
R=jamescook@chromium.org,sky@chromium.org

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

[modify] https://crrev.com/b53df9e2ff6a08ab6203129e8be41583712099f9/ash/shelf/shelf_tooltip_manager.cc
[modify] https://crrev.com/b53df9e2ff6a08ab6203129e8be41583712099f9/ash/shelf/shelf_tooltip_manager.h

Comment 9 by msw@chromium.org, May 4 2016

Status: Fixed (was: Started)

Sign in to add a comment