New issue
Advanced search Search tips

Issue 889884 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 5
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-10-08
OS: iOS
Pri: 1
Type: Bug-Regression



Sign in to add a comment

No Action performed on tapping the bottom links of the page

Project Member Reported by rakurati@chromium.org, Sep 27

Issue description

App Version: 70.0.3538.37 Beta
iOS Version: 10.3.3, 11.4.1, 12.0, 12.1 beta
Device: iPhones only

Steps to reproduce:
1. Launch chrome 
2. Load techmeme.com
3. Scroll to the end of the page
4. Tap on very bottom link like full site, mini and Top.

Observed results:
Nothing happens on tapping on the links at the bottom of the page

Note: Links only respond after scrolling the page up slightly and scroll down 

Expected results:
Target page should load

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: Not tested
Bug reproducible on Safari/Firefox: Firefox: No, Safari: No
Bug reproducible on current stable build (App Version, iOS Version): No on M69 
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M70

Link to video:
https://drive.google.com/file/d/1DNPJj1kl4Psf5qhbMH-jS7wVJJiky5TS/view?usp=sharing

 
Labels: -Pri-2 Pri-1
Labels: ReleaseBlock-Stable M-70
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
It is indeed not working. I think it might be related to the feature when the bottom toolbar is showing when you reach the bottom of the page.
Hi Kurt - ptal. Stable cut is next week.
Status: Started (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 5

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

commit 186999eebb6f82ea05fc60a5a19d0b9ae1cbed01
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Fri Oct 05 18:51:29 2018

[iOS] Fix touch handling for links at bottom of page.

This CL updates the |-hitTest:withEvent:| implementation for the toolbar
container view such that touches are ignored when they occur within the
container, but not on an actual toolbar.  This allows touches to reach
the web content area when the toolbars are collapsed and the content is
laid out behind the container.

Bug:  889884 
Change-Id: Ifbb6aff819784c692da9fdd649febfe51688e9f7
Reviewed-on: https://chromium-review.googlesource.com/c/1259882
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597216}
[modify] https://crrev.com/186999eebb6f82ea05fc60a5a19d0b9ae1cbed01/ios/chrome/browser/ui/browser_view_controller.mm

Cc: kariahda@chromium.org
NextAction: 2018-10-08
Status: Fixed (was: Started)
Fix has landed.  Setting a next action date for canary verification so we can cherry-pick.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-70; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-70 label, otherwise remove Merge-TBD label. Thanks.
The NextAction date has arrived: 2018-10-08
Status: Verified (was: Fixed)
Verified on M71.0.3573.0 canary iOS:12.1 iPhoneX
Able to tap links from the bottom of the page.
What's the likelihood this fix could brake something else? Is this fix behind a flag?
Labels: Merge-Request-70
The original fix was not behind a flag, but I can create one if you want, since this is a very late fix for this branch.  FWIW, the risk for this change is pretty minimal.
Project Member

Comment 13 by sheriffbot@chromium.org, Oct 8

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: Less than 4 days to go before AppStore submit on M70
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Is this only for techmeme.com or for all websites that have similar bottom links?

Yes, please to the flag since stable cut is this Thursday.
Cc: eugene...@chromium.org
Flag is added here: crrev.com/c/1269462, should be landed later today.

This affects any link at the bottom of the page, not just techmeme.  This will only occur when the link is behind where the bottom toolbar is normally laid out; if the user scrolls down again, such that the bottom toolbar is expanded again, the links are tappable without the fix.
Labels: -Merge-TBD -Merge-Review-70 Merge-Approved-70
Ok, approved pending flag CL landing. Please merge today if possible (after flag CL lands) so this can make this week's beta.
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 8

Project Member

Comment 18 by bugdroid1@chromium.org, Oct 8

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f931a2dc71b50099117506ecfd1abc0cddc796cd

commit f931a2dc71b50099117506ecfd1abc0cddc796cd
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Mon Oct 08 22:29:38 2018

[iOS] Fix touch handling for links at bottom of page.

This CL updates the |-hitTest:withEvent:| implementation for the toolbar
container view such that touches are ignored when they occur within the
container, but not on an actual toolbar.  This allows touches to reach
the web content area when the toolbars are collapsed and the content is
laid out behind the container.

Bug:  889884 
Change-Id: Ifbb6aff819784c692da9fdd649febfe51688e9f7
Reviewed-on: https://chromium-review.googlesource.com/c/1259882
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#597216}(cherry picked from commit 186999eebb6f82ea05fc60a5a19d0b9ae1cbed01)
Reviewed-on: https://chromium-review.googlesource.com/c/1269809
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#907}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/f931a2dc71b50099117506ecfd1abc0cddc796cd/ios/chrome/browser/ui/browser_view_controller.mm

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/f931a2dc71b50099117506ecfd1abc0cddc796cd

Commit: f931a2dc71b50099117506ecfd1abc0cddc796cd
Author: kkhorimoto@chromium.org
Commiter: kkhorimoto@chromium.org
Date: 2018-10-08 22:29:38 +0000 UTC

[iOS] Fix touch handling for links at bottom of page.

This CL updates the |-hitTest:withEvent:| implementation for the toolbar
container view such that touches are ignored when they occur within the
container, but not on an actual toolbar.  This allows touches to reach
the web content area when the toolbars are collapsed and the content is
laid out behind the container.

Bug:  889884 
Change-Id: Ifbb6aff819784c692da9fdd649febfe51688e9f7
Reviewed-on: https://chromium-review.googlesource.com/c/1259882
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#597216}(cherry picked from commit 186999eebb6f82ea05fc60a5a19d0b9ae1cbed01)
Reviewed-on: https://chromium-review.googlesource.com/c/1269809
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#907}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/de70831204e1f03e3a374b44986c5d2f012b4273

Commit: de70831204e1f03e3a374b44986c5d2f012b4273
Author: kkhorimoto@chromium.org
Commiter: kkhorimoto@chromium.org
Date: 2018-10-09 00:12:56 +0000 UTC

[iOS] Add flag for custom toolbar container touch handling.

TBR=kkhorimoto@chromium.org

(cherry picked from commit 0782b10d2dd32b3427c05cc27febf1dceaadf17e)

Bug:  889884 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ib63c457a9e122c3411b667086c2bbf1dc40f5c8c
Reviewed-on: https://chromium-review.googlesource.com/c/1269462
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#597705}
Reviewed-on: https://chromium-review.googlesource.com/c/1270178
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#909}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 9

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

commit de70831204e1f03e3a374b44986c5d2f012b4273
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Tue Oct 09 00:12:56 2018

[iOS] Add flag for custom toolbar container touch handling.

TBR=kkhorimoto@chromium.org

(cherry picked from commit 0782b10d2dd32b3427c05cc27febf1dceaadf17e)

Bug:  889884 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Ib63c457a9e122c3411b667086c2bbf1dc40f5c8c
Reviewed-on: https://chromium-review.googlesource.com/c/1269462
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#597705}
Reviewed-on: https://chromium-review.googlesource.com/c/1270178
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#909}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/de70831204e1f03e3a374b44986c5d2f012b4273/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/de70831204e1f03e3a374b44986c5d2f012b4273/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[modify] https://crrev.com/de70831204e1f03e3a374b44986c5d2f012b4273/ios/chrome/browser/ios_chrome_flag_descriptions.h
[modify] https://crrev.com/de70831204e1f03e3a374b44986c5d2f012b4273/ios/chrome/browser/ui/browser_view_controller.mm
[add] https://crrev.com/de70831204e1f03e3a374b44986c5d2f012b4273/ios/chrome/browser/ui/toolbar_container/toolbar_container_features.h
[add] https://crrev.com/de70831204e1f03e3a374b44986c5d2f012b4273/ios/chrome/browser/ui/toolbar_container/toolbar_container_features.mm

Verified on chrome beta version 70.0.3538.60 following the steps mentioned in comment#0.   Bottom links are tappable.  Looks good.

Devices: iPhone X, iPhone 6S plus, iPhone 7 plus.
iOS:  11.4.1, 12.1 beta 3, 10.3.3

This issues has been verified and found fixed. Details are given below:
Build No. : 71.0.3578.8
Device: iPhone8 plus, iPhone SE, Iphone7 plus
OS Version: 11.4.1, 12.0.1, 

Sign in to add a comment