New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 900632 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: ----



Sign in to add a comment

Launcher button seems unresponsive

Project Member Reported by newcomer@chromium.org, Oct 31

Issue description

Chrome Version: 72.0.3593.0

What steps will reproduce the problem?
(1) Show shelf.
(2) Tap the launcher button.

What is the expected result?
Launcher easily shows up on the first try.

What happens instead?
Initial investigation shows that the launcher button is not receiving these events, they are going the shelf. It feels like I am tapping the button though.

Maybe we should give this button a larger touch target, or give more margin for error for this button. It is tricky to press sometimes, see the repro video.

Please note, I'm not sure how long this has been this way, or if my thumb is just not shaped normally.

Video:
https://photos.app.goo.gl/mKJfMyV4igccErCz8
 
A listnr report of the same issue.
https://critique.corp.google.com/#review/219563694
I did some investigations here, it seems like ShelfView::OnGestureEvent is called even when the event bounds are within the app list buttons bounds.

I can't repro this on the emulator, only on device.
Issue 894653 has been merged into this issue.
Owner: newcomer@chromium.org
Status: Started (was: Available)
Cc: manucornet@chromium.org
Looks like the launcher button went from the old size, 48, up to 56 temporarily, then back down to 40.

I'm guessing 40 is just a bit too small.
Looks like the actual cause of the bug is the nefarious back_and_app_list_background_ which was stealing events before they could get to the AppListButton.
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 16

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

commit 879b37c6c751745d8c9d9dcf67e1f5958991307f
Author: Alex Newcomer <newcomer@chromium.org>
Date: Fri Nov 16 16:34:29 2018

cros: Fix unresponsive app list button

|back_and_app_list_background_| is interfering with event targeting and
causing the app list button to be unresponsive, specifically in the
right side of the button. NOTE: This only repros on device, not on the
emulator.

Prevent |back_and_app_list_background_| from recieving events to prevent
this.

Bug:  900632 
Change-Id: Ib2cd8e51cb560d84307904c86ae46ecc388e2424
Reviewed-on: https://chromium-review.googlesource.com/c/1338723
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608796}
[modify] https://crrev.com/879b37c6c751745d8c9d9dcf67e1f5958991307f/ash/shelf/shelf_view.cc

Cc: kaznacheev@chromium.org kbleicher@chromium.org
Labels: Merge-Request-71
I know it's late, but this is a huge quality of life improvement (especially on nocturne), and it is a very safe 1 line fix.
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 17

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-71 Merge-Approved-71
Approved for M71 ChromeOS.
Labels: -Merge-Approved-71 Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/73ef1380dda8ccb4e14845a1692cb095a416aa46

Commit: 73ef1380dda8ccb4e14845a1692cb095a416aa46
Author: newcomer@chromium.org
Commiter: newcomer@chromium.org
Date: 2018-11-19 16:43:03 +0000 UTC

cros: Fix unresponsive app list button

|back_and_app_list_background_| is interfering with event targeting and
causing the app list button to be unresponsive, specifically in the
right side of the button. NOTE: This only repros on device, not on the
emulator.

Prevent |back_and_app_list_background_| from recieving events to prevent
this.

Bug:  900632 
Change-Id: Ib2cd8e51cb560d84307904c86ae46ecc388e2424
Reviewed-on: https://chromium-review.googlesource.com/c/1338723
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#608796}(cherry picked from commit 879b37c6c751745d8c9d9dcf67e1f5958991307f)
Reviewed-on: https://chromium-review.googlesource.com/c/1342398
Cr-Commit-Position: refs/branch-heads/3578@{#751}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 19

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73ef1380dda8ccb4e14845a1692cb095a416aa46

commit 73ef1380dda8ccb4e14845a1692cb095a416aa46
Author: Alex Newcomer <newcomer@chromium.org>
Date: Mon Nov 19 16:43:03 2018

cros: Fix unresponsive app list button

|back_and_app_list_background_| is interfering with event targeting and
causing the app list button to be unresponsive, specifically in the
right side of the button. NOTE: This only repros on device, not on the
emulator.

Prevent |back_and_app_list_background_| from recieving events to prevent
this.

Bug:  900632 
Change-Id: Ib2cd8e51cb560d84307904c86ae46ecc388e2424
Reviewed-on: https://chromium-review.googlesource.com/c/1338723
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Alex Newcomer <newcomer@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#608796}(cherry picked from commit 879b37c6c751745d8c9d9dcf67e1f5958991307f)
Reviewed-on: https://chromium-review.googlesource.com/c/1342398
Cr-Commit-Position: refs/branch-heads/3578@{#751}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/73ef1380dda8ccb4e14845a1692cb095a416aa46/ash/shelf/shelf_view.cc

Status: Fixed (was: Started)

Sign in to add a comment