New issue
Advanced search Search tips

Issue 883694 link

Starred by 1 user
Status: Verified
Owner:
gambard@chromium.org
Closed: Sep 21
Cc:
kariahda@chromium.org
kkhorimoto@chromium.org
ios-bugs-priority@chromium.org
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Impossible to swipe to change tab on the bottom toolbar.

Project Member Reported by gambard@chromium.org, Sep 13 
Chrome Version: M71
phone only

What steps will reproduce the problem?
(1) Swipe on the bottom toolbar

What is the expected result?
It should be possible to change tabs.

What happens instead?
Nothing.

Probably related to  issue 878348 .

Comment 1 by gambard@chromium.org, Sep 13

Cc: kkhorimoto@chromium.org
Actually, it was introduced by crrev.com/c/1208713.
kkhorimoto@: please take it into account for the new toolbar container.

Comment 2 by kkhorimoto@chromium.org, Sep 13

Cc: -kkhorimoto@chromium.org gambard@chromium.org
Labels: -M-71 M-70
Owner: kkhorimoto@chromium.org
Status: Started (was: Assigned)
I'll take a look at this today since it was merged into M70.

Comment 4 by kkhorimoto@chromium.org, Sep 13

Cc: -gambard@chromium.org kkhorimoto@chromium.org
Owner: gambard@chromium.org
Kicking back to Gauthier since he already has a fix here: crrev.com/c/1224387

Comment 5 by gambard@chromium.org, Sep 14 

Sorry, I forgot to update the bug when I did the fix :/
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 18 

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

commit 92d8795b92e83d185aa52f4ad8b43c02d93e4639
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Sep 18 13:06:11 2018

[iOS] Check SideSwipe location in window coordinates

This CL changes the way the SideSwipe is checking if the touch location
is inside the toolbar frame or not by passing it in the window
coordinates instead of in the gesture's view coordinates.
This starts creating an issue as the toolbar frame is now contained in
a container, changing its frame's origin.

Bug:  883694 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: Iaf42b36ce01c47addd87ae71772abff605e75a1c
Reviewed-on: https://chromium-review.googlesource.com/1224387
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592014}
[modify] https://crrev.com/92d8795b92e83d185aa52f4ad8b43c02d93e4639/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/92d8795b92e83d185aa52f4ad8b43c02d93e4639/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[add] https://crrev.com/92d8795b92e83d185aa52f4ad8b43c02d93e4639/ios/chrome/browser/ui/side_swipe/side_swipe_egtest.mm
[modify] https://crrev.com/92d8795b92e83d185aa52f4ad8b43c02d93e4639/ios/chrome/browser/ui/toolbar/adaptive/toolbar_coordinator_adaptor.mm
[modify] https://crrev.com/92d8795b92e83d185aa52f4ad8b43c02d93e4639/ios/chrome/browser/ui/toolbar/public/side_swipe_toolbar_interacting.h
[modify] https://crrev.com/92d8795b92e83d185aa52f4ad8b43c02d93e4639/ios/chrome/test/earl_grey/BUILD.gn
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 18 

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

commit 75801edb5336100c7e18af806a08b763dda72bdd
Author: edchin <edchin@chromium.org>
Date: Tue Sep 18 20:13:47 2018

Revert "[iOS] Check SideSwipe location in window coordinates"

This reverts commit 92d8795b92e83d185aa52f4ad8b43c02d93e4639.

Reason for revert:

Failures on iOS 10 devices and simulator bots:
ipad10-device-x64
ipad10-simulator-x64

ios_chrome_ui_egtests (iPad Air iOS 10.3.3) on iOS-10.3.3


SideSwipeTestCase/testSideSwipeTopToolbar:
[0918/100933.388949:WARNING:embedded_test_server.cc(239)] Request not handled. Returning 404: /favicon.ico
../../ios/chrome/test/earl_grey/chrome_earl_grey.mm:188: error: -[SideSwipeTestCase testSideSwipeTopToolbar] : Exception: AssertionFailedException
Exception Name: AssertionFailedException
Exception Reason: (([condition waitWithTimeout:base::test::ios::kWaitForUIElementTimeout]) is true) failed
Exception Details: Failed waiting for web view containing Echo
Bundle ID: com.google.gtest.ios-chrome-ui-egtests
Stack Trace: (
	0   EarlGrey                            0x0000000105c58748 -[GREYDefaultFailureHandler handleException:details:] + 780
	1   ios_chrome_ui_egtests               0x0000000100a05cc8 +[ChromeEarlGrey waitForWebViewContainingText:] + 628
	2   ios_chrome_ui_egtests               0x0000000100a543fc -[SideSwipeTestCase checkSideSwipeOnToolbarClass:] + 1196
	3   CoreFoundation                      0x0000000187ea4e80 <redacted> + 144
	4   CoreFoundation                      0x0000000187d9a2c4 <redacted> + 292
	5   EarlGrey                            0x0000000105c495c8 -[GREYTestCaseInvocation invoke] + 84
	6   XCTest                              0x0000000105aacf3c __24-[XCTestCase invokeTest]_block_invoke_2.198 + 68
	7   XCTest                              0x0000000105b14560 -[XCTMemoryChecker _assertInvalidObjectsDeallocatedAfterScope:] + 68
	8   XCTest                              0x0000000105ab57c0 -[XCTestCase assertInvalidObjectsDeallocatedAfterScope:] + 112
	9   XCTest                              0x0000000105aacec4 __24-[XCTestCase invokeTest]_block_invoke.192 + 196
	10  XCTest                              0x0000000105b01710 -[XCTestCase(Failures) performFailableBlock:testCaseRun:shouldInterruptTest:] + 64
	11  XCTest                              0x0000000105b01628 -[XCTestCase(Failures) _performTurningExceptionsIntoFailuresInterruptAfterHandling:block:] + 108
	12  XCTest                              0x0000000105aacb38 __24-[XCTestCase invokeTest]_block_invoke + 812
	13  XCTest                              0x0000000105b07298 -[XCUITestContext performInScope:] + 264
	14  XCTest                              0x0000000105aac744 -[XCTestCase testContextPerformInScope:] + 100
	15  XCTest                              0x0000000105aac7f8 -[XCTestCase invokeTest] + 140
	16  EarlGrey                            0x0000000105c40e6c -[XCTestCase(GREYAdditions) grey_invokeTest] + 392
	17  XCTest                              0x0000000105aae39c __26-[XCTestCase performTest:]_block_invoke_2 + 44
	18  XCTest                              0x0000000105b01710 -[XCTestCase(Failures) performFailableBlock:testCaseRun:shouldInterruptTest:] + 64
	19  XCTest                              0x0000000105b01628 -[XCTestCase(Failures) _performTurningExceptionsIntoFailuresInterruptAfterHandling:block:] + 108
	20  XCTest                              0x0000000105aae2b4 __26-[XCTestCase performTest:]_block_invoke.337 + 96
	21  XCTest                              0x0000000105b11a58 +[XCTContext runInContextForTestCase:block:] + 216
	22  XCTest                              0x0000000105aada50 -[XCTestCase performTest:] + 648
	23  XCTest                              0x0000000105aee88c -[XCTest runTest] + 60
	24  XCTest                              0x0000000105aa8ec4 __27-[XCTestSuite performTest:]_block_invoke + 296
	25  XCTest                              0x0000000105aa87c4 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 68
	26  XCTest                              0x0000000105aa8a5c -[XCTestSuite performTest:] + 272
	27  XCTest                              0x0000000105aee88c -[XCTest runTest] + 60
	28  XCTest                              0x0000000105aa8ec4 __27-[XCTestSuite performTest:]_block_invoke + 296
	29  XCTest                              0x0000000105aa87c4 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 68
	30  XCTest                              0x0000000105aa8a5c -[XCTestSuite performTest:] + 272
	31  XCTest                              0x0000000105aee88c -[XCTest runTest] + 60
	32  XCTest                              0x0000000105aa8ec4 __27-[XCTestSuite performTest:]_block_invoke + 296
	33  XCTest                              0x0000000105aa87c4 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 68
	34  XCTest                              0x0000000105aa8a5c -[XCTestSuite performTest:] + 272
	35  XCTest                              0x0000000105aee88c -[XCTest runTest] + 60
	36  XCTest                              0x0000000105b1eb2c __44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke + 176
	37  XCTest                              0x0000000105b1ec5c __44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke.80 + 60
	38  XCTest                              0x0000000105ac08fc -[XCTestObservationCenter _observeTestExecutionForBlock:] + 524
	39  XCTest                              0x0000000105b1e8ac -[XCTTestRunSession runTestsAndReturnError:] + 612
	40  XCTest                              0x0000000105a8dad4 -[XCTestDriver runTestsAndReturnError:] + 416
	41  XCTest                              0x0000000105b0e274 _XCTestMain + 1436
	42  libXCTestBundleInject.dylib         0x0000000105915724 __copy_helper_block_ + 0
	43  CoreFoundation                      0x0000000187e4d30c <redacted> + 20
	44  CoreFoundation                      0x0000000187e4cb28 <redacted> + 288
	45  CoreFoundation                      0x0000000187e4a998 <redacted> + 728
	46  CoreFoundation                      0x0000000187d7ada4 CFRunLoopRunSpecific + 424
	47  GraphicsServices                    0x00000001897e5074 GSEventRunModal + 100
	48  UIKit                               0x000000018e02ec9c UIApplicationMain + 208
	49  ios_chrome_ui_egtests               0x0000000100045618 main + 536
	50  libdyld.dylib                       0x0000000186d8959c <redacted> + 4
)
Screenshots: {
  "Screenshot At Failure":  "/var/mobile/Containers/Data/Application/8AE3CF5E-F002-4373-8F6F-826C6E868992/Documents/SideSwipeTestCase_testSideSwipeTopToolbar-AssertionFailedException-001584B5-0BD5-480A-BE35-237D1281481F/SideSwipeTestCase_testSideSwipeTopToolbar.png",
  "Visibility Checker Most Recent Before Image":  "/var/mobile/Containers/Data/Application/8AE3CF5E-F002-4373-8F6F-826C6E868992/Documents/SideSwipeTestCase_testSideSwipeTopToolbar-AssertionFailedException-001584B5-0BD5-480A-BE35-237D1281481F/SideSwipeTestCase_testSideSwipeTopToolbar_before.png",
  "Visibility Checker Most Recent Expected After Image":  "/var/mobile/Containers/Data/Application/8AE3CF5E-F002-4373-8F6F-826C6E868992/Documents/SideSwipeTestCase_testSideSwipeTopToolbar-AssertionFailedException-001584B5-0BD5-480A-BE35-237D1281481F/SideSwipeTestCase_testSideSwipeTopToolbar_after_expected.png",
  "Visibility Checker Most Recent Actual After Image":  "/var/mobile/Containers/Data/Application/8AE3CF5E-F002-4373-8F6F-826C6E868992/Documents/SideSwipeTestCase_testSideSwipeTopToolbar-AssertionFailedException-001584B5-0BD5-480A-BE35-237D1281481F/SideSwipeTestCase_testSideSwipeTopToolbar_after_actual.png"
}
UI hierarchy (ordered by window level, front to back as rendered):
Legend: {
  "[AX]":  "Accessibility",
  "[UIE]":  "User Interaction Enabled",
  "[Window 1]":  "Frontmost Window"
}





Original change's description:
> [iOS] Check SideSwipe location in window coordinates
> 
> This CL changes the way the SideSwipe is checking if the touch location
> is inside the toolbar frame or not by passing it in the window
> coordinates instead of in the gesture's view coordinates.
> This starts creating an issue as the toolbar frame is now contained in
> a container, changing its frame's origin.
> 
> Bug:  883694 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
> Change-Id: Iaf42b36ce01c47addd87ae71772abff605e75a1c
> Reviewed-on: https://chromium-review.googlesource.com/1224387
> Commit-Queue: Gauthier Ambard <gambard@chromium.org>
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Reviewed-by: Justin Cohen <justincohen@chromium.org>
> Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#592014}

TBR=justincohen@chromium.org,eugenebut@chromium.org,kkhorimoto@chromium.org,gambard@chromium.org

Change-Id: I8ea07375dc2c910264e05ecd58ade762c99a582c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  883694 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/1231828
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592170}
[modify] https://crrev.com/75801edb5336100c7e18af806a08b763dda72bdd/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/75801edb5336100c7e18af806a08b763dda72bdd/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[delete] https://crrev.com/7140f23fd1268a38a1176ecc7ba39c9e107b0638/ios/chrome/browser/ui/side_swipe/side_swipe_egtest.mm
[modify] https://crrev.com/75801edb5336100c7e18af806a08b763dda72bdd/ios/chrome/browser/ui/toolbar/adaptive/toolbar_coordinator_adaptor.mm
[modify] https://crrev.com/75801edb5336100c7e18af806a08b763dda72bdd/ios/chrome/browser/ui/toolbar/public/side_swipe_toolbar_interacting.h
[modify] https://crrev.com/75801edb5336100c7e18af806a08b763dda72bdd/ios/chrome/test/earl_grey/BUILD.gn
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 20 

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

commit 4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9
Author: Gauthier Ambard <gambard@chromium.org>
Date: Thu Sep 20 09:35:48 2018

[iOS] Reland of "Check SideSwipe location in window coordinates"

This CL changes the way the SideSwipe is checking if the touch location
is inside the toolbar frame or not by passing it in the window
coordinates instead of in the gesture's view coordinates.
This starts creating an issue as the toolbar frame is now contained in
a container, changing its frame's origin.

Bug:  883694 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I6387b2bc68cbbe6b272a4cb88d080fe3dd96c870
Reviewed-on: https://chromium-review.googlesource.com/1233336
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592734}
[modify] https://crrev.com/4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[add] https://crrev.com/4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9/ios/chrome/browser/ui/side_swipe/side_swipe_egtest.mm
[modify] https://crrev.com/4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9/ios/chrome/browser/ui/toolbar/adaptive/toolbar_coordinator_adaptor.mm
[modify] https://crrev.com/4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9/ios/chrome/browser/ui/toolbar/public/side_swipe_toolbar_interacting.h
[modify] https://crrev.com/4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9/ios/chrome/test/earl_grey/BUILD.gn

Comment 9 by gambard@chromium.org, Sep 21

Cc: kariahda@chromium.org
Labels: Merge-Request-70
Status: Fixed (was: Started)
Verified on Canary. Asking for merge.
Project Member

Comment 10 by sheriffbot@chromium.org, Sep 21

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: Less than 21 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

Comment 11 by kariahda@chromium.org, Sep 21 

Is it just crrev.com/c/1233336 that needs to be merged? Why does this need to make M70?

Comment 12 by kkhorimoto@chromium.org, Sep 21 

This regression was introduced by my fix for the secondary toolbar animations, which was also merged into M70: crrev.com/c/1208713.

Comment 13 by kariahda@chromium.org, Sep 24

Labels: -Merge-Review-70 Merge-Approved-70
Approved.
Project Member

Comment 14 by bugdroid1@chromium.org, Sep 25

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

commit 9427c7f74395894afa7375b1ad2cb46d19f8a5b3
Author: Gauthier Ambard <gambard@chromium.org>
Date: Tue Sep 25 07:20:33 2018

[iOS] Reland of "Check SideSwipe location in window coordinates"

This CL changes the way the SideSwipe is checking if the touch location
is inside the toolbar frame or not by passing it in the window
coordinates instead of in the gesture's view coordinates.
This starts creating an issue as the toolbar frame is now contained in
a container, changing its frame's origin.

Bug:  883694 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I6387b2bc68cbbe6b272a4cb88d080fe3dd96c870
Reviewed-on: https://chromium-review.googlesource.com/1233336
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#592734}(cherry picked from commit 4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9)
Reviewed-on: https://chromium-review.googlesource.com/1242457
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#647}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/9427c7f74395894afa7375b1ad2cb46d19f8a5b3/ios/chrome/browser/ui/side_swipe/BUILD.gn
[modify] https://crrev.com/9427c7f74395894afa7375b1ad2cb46d19f8a5b3/ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm
[add] https://crrev.com/9427c7f74395894afa7375b1ad2cb46d19f8a5b3/ios/chrome/browser/ui/side_swipe/side_swipe_egtest.mm
[modify] https://crrev.com/9427c7f74395894afa7375b1ad2cb46d19f8a5b3/ios/chrome/browser/ui/toolbar/adaptive/toolbar_coordinator_adaptor.mm
[modify] https://crrev.com/9427c7f74395894afa7375b1ad2cb46d19f8a5b3/ios/chrome/browser/ui/toolbar/public/side_swipe_toolbar_interacting.h
[modify] https://crrev.com/9427c7f74395894afa7375b1ad2cb46d19f8a5b3/ios/chrome/test/earl_grey/BUILD.gn
Project Member

Comment 15 by cr-audit...@appspot.gserviceaccount.com, Sep 25

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

Commit: 9427c7f74395894afa7375b1ad2cb46d19f8a5b3
Author: gambard@chromium.org
Commiter: gambard@chromium.org
Date: 2018-09-25 07:20:33 +0000 UTC

[iOS] Reland of "Check SideSwipe location in window coordinates"

This CL changes the way the SideSwipe is checking if the touch location
is inside the toolbar frame or not by passing it in the window
coordinates instead of in the gesture's view coordinates.
This starts creating an issue as the toolbar frame is now contained in
a container, changing its frame's origin.

Bug:  883694 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I6387b2bc68cbbe6b272a4cb88d080fe3dd96c870
Reviewed-on: https://chromium-review.googlesource.com/1233336
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#592734}(cherry picked from commit 4e06d8b83e4be6afb02ab8f0fcc1ac0245e714b9)
Reviewed-on: https://chromium-review.googlesource.com/1242457
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#647}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}

Comment 16 by rakurati@chromium.org, Sep 25

Status: Verified (was: Fixed)
Verified in 
Build: 71.0.3561.0 Canary
Devices: iPhone 6(iOS 11), iPhoneX(iOS 12.0), iPhone 7plus(iOS 10.3.3)

Possible to swipe to change tab on bottom tool bar now, looks good.

Link to video:
https://drive.google.com/file/d/17dwtHdpt15JVkl5LxT0EnYtT7Q7Mg-0U/view?usp=sharing

Comment 17 by vbarig...@chromium.org, Oct 3 

Verified in on chrome beta version 70.0.3538.44 on iPhone 6s plus and iPhoneX with 12, iPhone 7 plus with iOS 10.3.3, following steps mentioned in comment #0.  Possible to change tabs by swiping on tool bar.  Looks good.
►

Sign in to add a comment