New issue
Advanced search Search tips

Issue 753723 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 730232



Sign in to add a comment

Toolbar menus doesn't close upon processed tap / longtap / longpress

Project Member Reported by yamaguchi@chromium.org, Aug 9 2017

Issue description

Known issue with patch
https://chromium-review.googlesource.com/c/606068

Steps To Reproduce:
(1) Tap the sort button [AZ] in the tool bar. See the menu pops up.
(2) Long-tap an item

Expected Result:
The sort menu closes.

Actual Result:
The sort menu stays.
The file is selected.

 
Owner: yamaguchi@chromium.org
Status: Started (was: Unconfirmed)
The change https://chromium-review.googlesource.com/c/608347 will fix this.
Blocking: 730232
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 14 2017

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

commit 0a47b209521568489f0cfba7f39fb0e8bdec34b4
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Mon Aug 14 09:25:08 2017

Close a menu when tapping outside it, in addition to mouse click.

This is needed when the app processes touch events and cancels it so as
to not trigger mousedown events after touch.

Bug:  753723 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I37bab3186ebb982a56339633c8898827e0dfd478
Reviewed-on: https://chromium-review.googlesource.com/608347
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494024}
[modify] https://crrev.com/0a47b209521568489f0cfba7f39fb0e8bdec34b4/ui/webui/resources/js/cr/ui/menu_button.js

Labels: Merge-Request-61
Status: Fixed (was: Started)
Labels: -Merge-Request-61 Merge-Approved-61
Approving merge for M61 Chrome OS.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 15 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ea5b1ed2f77790dcf1012131193b1fb30715b95

commit 6ea5b1ed2f77790dcf1012131193b1fb30715b95
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Tue Aug 15 06:05:41 2017

Close a menu when tapping outside it, in addition to mouse click.

This is needed when the app processes touch events and cancels it so as
to not trigger mousedown events after touch.

TBR=yamaguchi@chromium.org

(cherry picked from commit 0a47b209521568489f0cfba7f39fb0e8bdec34b4)

Bug:  753723 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I37bab3186ebb982a56339633c8898827e0dfd478
Reviewed-on: https://chromium-review.googlesource.com/608347
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#494024}
Reviewed-on: https://chromium-review.googlesource.com/614939
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#569}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/6ea5b1ed2f77790dcf1012131193b1fb30715b95/ui/webui/resources/js/cr/ui/menu_button.js

Status: Assigned (was: Fixed)
Reopneing, as this reproduces on ToT.
Commit 0a47b209521568489f0cfba7f39fb0e8bdec34b4 didn't fix the issue and caused another issue. "The menu sometimes don't close by tapping the menu button again." So I am reverting the change.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 15 2017

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

commit 636d1e0453bdc143ded019bb3e45331a10c6dfa3
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Tue Aug 15 14:41:20 2017

Revert "Close a menu when tapping outside it, in addition to mouse click."

This reverts commit 0a47b209521568489f0cfba7f39fb0e8bdec34b4.

Reason for revert: The change didn't fix 753723, and also caused a regression bug (crbug/753723#c9).

Original change's description:
> Close a menu when tapping outside it, in addition to mouse click.
> 
> This is needed when the app processes touch events and cancels it so as
> to not trigger mousedown events after touch.
> 
> Bug:  753723 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: I37bab3186ebb982a56339633c8898827e0dfd478
> Reviewed-on: https://chromium-review.googlesource.com/608347
> Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#494024}

TBR=michaelpg@chromium.org,yamaguchi@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  753723 
Change-Id: I577d20e4f1686c61b78597008149bfd22a556356
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/615323
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494377}
[modify] https://crrev.com/636d1e0453bdc143ded019bb3e45331a10c6dfa3/ui/webui/resources/js/cr/ui/menu_button.js

Labels: Merge-Request-61
I'd like to merge 636d1e0453bdc143ded019bb3e45331a10c6dfa3 to M61 by the reason described above. It will revert the mergeed change 6ea5b1ed2f77790dcf1012131193b1fb30715b95 as described in #9 and #10.
Project Member

Comment 12 by sheriffbot@chromium.org, Aug 16 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 Chrome OS.
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 17 2017

Labels: -merge-approved-61
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e3f0e808d360ae3a7fbfd2ace613253abb71b4b

commit 5e3f0e808d360ae3a7fbfd2ace613253abb71b4b
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Thu Aug 17 01:46:41 2017

Revert "Close a menu when tapping outside it, in addition to mouse click."

This reverts commit 0a47b209521568489f0cfba7f39fb0e8bdec34b4.

Reason for revert: The change didn't fix 753723, and also caused a regression bug (crbug/753723#c9).

Original change's description:
> Close a menu when tapping outside it, in addition to mouse click.
>
> This is needed when the app processes touch events and cancels it so as
> to not trigger mousedown events after touch.
>
> Bug:  753723 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: I37bab3186ebb982a56339633c8898827e0dfd478
> Reviewed-on: https://chromium-review.googlesource.com/608347
> Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#494024}

TBR=michaelpg@chromium.org, yamaguchi@chromium.org


(cherry picked from commit 636d1e0453bdc143ded019bb3e45331a10c6dfa3)

Bug:  753723 
Change-Id: I577d20e4f1686c61b78597008149bfd22a556356
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/615323
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#494377}
Reviewed-on: https://chromium-review.googlesource.com/617847
Cr-Commit-Position: refs/branch-heads/3163@{#623}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/5e3f0e808d360ae3a7fbfd2ace613253abb71b4b/ui/webui/resources/js/cr/ui/menu_button.js

Status: Started (was: Assigned)
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 21 2017

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

commit 8394bae7034e5eeb9bf5ada381fc3ea39baf323a
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Mon Aug 21 03:26:05 2017

Allow to close menu by tapping outside it.

Allow to close menu by tapping outside it, in addition to mouse click.
This a revised version of 608347 after fixing the issues.
Some UI components can cancel events after processing them. Therefore
the decorated object may not receive touchstart event in such case.

Bug:  753723 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I168e59bc9649d91a9bc3b1c11795153fc2547093
Reviewed-on: https://chromium-review.googlesource.com/616404
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495869}
[modify] https://crrev.com/8394bae7034e5eeb9bf5ada381fc3ea39baf323a/ui/webui/resources/js/cr/ui/menu_button.js

Labels: Merge-Request-61
Status: Fixed (was: Started)
Labels: -Merge-Request-61 Merge-Approved-61
Approving merge to M61 Chrome OS. yamaguchi@ Please request for approval before you merge into the branch.
Project Member

Comment 19 by bugdroid1@chromium.org, Aug 22 2017

Labels: -merge-approved-61
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b76f568431c46f8483fd32ae4f94a5f52bc9a643

commit b76f568431c46f8483fd32ae4f94a5f52bc9a643
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Tue Aug 22 01:46:32 2017

Allow to close menu by tapping outside it.

Allow to close menu by tapping outside it, in addition to mouse click.
This a revised version of 608347 after fixing the issues.
Some UI components can cancel events after processing them. Therefore
the decorated object may not receive touchstart event in such case.

TBR=yamaguchi@chromium.org

(cherry picked from commit 8394bae7034e5eeb9bf5ada381fc3ea39baf323a)

Bug:  753723 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I168e59bc9649d91a9bc3b1c11795153fc2547093
Reviewed-on: https://chromium-review.googlesource.com/616404
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#495869}
Reviewed-on: https://chromium-review.googlesource.com/624765
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#747}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/b76f568431c46f8483fd32ae4f94a5f52bc9a643/ui/webui/resources/js/cr/ui/menu_button.js

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

Status: Archived (was: Fixed)

Sign in to add a comment