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

Issue 776662 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

[Bling Bookmarks] Tapping outside the button text triggers button action.

Project Member Reported by martiw@chromium.org, Oct 20 2017

Issue description

App Version: 64.0.3240.0 canary, (should happen in 63.0.3239 too)
iOS Version: 11.1
Device: iPhone, iPad

Precondition:
1. Go to chrome://flags --> Bookmark New Generation iOS : Enabled
2. has some bookmarks.

Steps to reproduce:
  1.  Launch chrome 
  2.  Go to Menu -->  Bookmarks -> Mobile Bookmarks.
  3.  tap on the left of "Select" button.

Observed results:
  "Select" button action is triggered.

Expected results:
  it should not trigger button action.

Root cause:
  The buttons (tappable area) occupies the entire toolbar.
  The buttons should enclose the texts only.

Solution:
  Create a UIView as the button container (like what reading list did)

I am working on the fix right now.
 

Comment 1 by martiw@chromium.org, Oct 20 2017

Labels: OS-iOS
Cc: gambard@chromium.org
+ gambard@

The tappable areas should be similar to what we have in Reading List and History.
Components: UI>Browser>Bookmarks
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 21 2017

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

commit 913589224ca81e424515bb180e9380495576b3b9
Author: Marti Wong <martiw@chromium.org>
Date: Sat Oct 21 03:57:17 2017

Fix the tappable area of bottom bar buttons (new iOS bookmark)

Make some UIViews to contain the buttons of the stackView in the bottom
toolbar.  This way the buttons will contain the button text only and no
longer occupy the entire stackView. This is to prevent the buttons from
being tappable outside of the button text.

Bug:  776662 
Change-Id: I7eff688704c1239c7507c5f1bc9afe113754d118
Reviewed-on: https://chromium-review.googlesource.com/732044
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marti Wong <martiw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510657}
[modify] https://crrev.com/913589224ca81e424515bb180e9380495576b3b9/ios/chrome/browser/ui/bookmarks/bars/bookmark_context_bar.mm

Comment 5 by martiw@chromium.org, Oct 22 2017

Labels: Merge-Request-63
Status: Fixed (was: Assigned)

Comment 6 by martiw@chromium.org, Oct 23 2017

Cc: martiw@chromium.org
 Issue 777256  has been merged into this issue.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 23 2017

Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 23 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ba708326cd5cb667f6c262aad59e2191ab734ab

commit 0ba708326cd5cb667f6c262aad59e2191ab734ab
Author: Marti Wong <martiw@chromium.org>
Date: Mon Oct 23 23:47:50 2017

Fix the tappable area of bottom bar buttons (new iOS bookmark)

Make some UIViews to contain the buttons of the stackView in the bottom
toolbar.  This way the buttons will contain the button text only and no
longer occupy the entire stackView. This is to prevent the buttons from
being tappable outside of the button text.

Bug:  776662 
Change-Id: I7eff688704c1239c7507c5f1bc9afe113754d118
Reviewed-on: https://chromium-review.googlesource.com/732044
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marti Wong <martiw@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#510657}(cherry picked from commit 913589224ca81e424515bb180e9380495576b3b9)
Reviewed-on: https://chromium-review.googlesource.com/734062
Reviewed-by: Marti Wong <martiw@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#170}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/0ba708326cd5cb667f6c262aad59e2191ab734ab/ios/chrome/browser/ui/bookmarks/bars/bookmark_context_bar.mm

Status: Verified (was: Fixed)
Verified on iPhone 6+ iOS 11.1 , iPad Pro 12'5 iOS 11.1, iPhone 7+ iOS 10.3.3
on build 64.0.3248.0 canary.
Verified on chrome beta version 63.0.3239.19, following the steps mentioned in comment #0.  Select button and other buttons on New Bookmarks UI triggered when tapped exactly on the text.  Looks good.

Devices: iPhone 6s plus, iPad Air
iOS: 10.3.3, 11.0





Sign in to add a comment