New issue
Advanced search Search tips

Issue 869820 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 1
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-08-02
OS: iOS
Pri: 1
Type: Bug
Q2



Sign in to add a comment

App crashes on loading new tab page using keyboard shortcut (cmd + T)

Project Member Reported by rakurati@chromium.org, Aug 1

Issue description

App Version: 70.0.3508.0 Canary
iOS Version: 10.3.3, 11.4.1, 12 beta 5
Device: iPhone and iPad

Precondition: connect the bluetooth keyboard to the device

Steps to reproduce:
1. Launch chrome 
2. Close all incognito tabs if there is any open
3. Go to tab grid recent tabs 
4. Press cmd+T 

Observed results:
Notice that app crashes

Expected results:
App shouldn’t crash a new regular tab should be opened

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

Link to video:
https://drive.google.com/file/d/1lWjcgLJX-eeynvK6GhE--M7C4PxDuc_o/view?usp=sharing

 
Labels: M-69 Q2
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1

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

commit 512743d1acf7b809250f4aae884b30428037446f
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Wed Aug 01 19:32:12 2018

[iOS] Ignore command+T from remote tabs page in tab grid.

|-openNewTabInPage:focusOmnibox:| was a no-op if the page was the
remote tabs  page, but TabGridCoordinator's |-showActiveTabInPage:
focusOmnibox:| used a NOTREACH() when called with the remote tabs page.
This CL updates them both to use NOTREACH(), and short-circuits from the
command+T handler if the page is the remote tabs page.

Bug:  869820 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib394cc37a2b70420d3611e1440ad2b628a27c0a6
Reviewed-on: https://chromium-review.googlesource.com/1159087
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579901}
[modify] https://crrev.com/512743d1acf7b809250f4aae884b30428037446f/ios/chrome/browser/ui/tab_grid/tab_grid_view_controller.mm

Labels: Merge-TBD
NextAction: 2018-08-02
Setting next action date for canary verification.
Status: Fixed (was: Started)
The NextAction date has arrived: 2018-08-02
Cc: shbarezer@chromium.org kariahda@chromium.org
I've verified this on simulator by pulling ToT, but I don't have a bluetooth keyboard with which to test this on canary.  Sharon, could you verify this fix so it can be merged?  Thanks!
Status: Verified (was: Fixed)
M70.0.3515.0 canary
iOS12.0 beta#6, iPad Pro 10.5"

App doesn't crash after performing the steps from original report
Labels: Merge-Rejected-69
Thanks for verifying, Srikanth!
Labels: -Merge-Rejected-69 Merge-Request-69
Putting correct label.
Project Member

Comment 14 by sheriffbot@chromium.org, Aug 7

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Less than 24 days to go before AppStore submit on M69
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-TBD -Merge-Review-69 Merge-Approved-69
Approved.
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 7

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff004551655b0ba11c28c884bdcb5047aa1bfda4

commit ff004551655b0ba11c28c884bdcb5047aa1bfda4
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Tue Aug 07 22:02:57 2018

[iOS] Ignore command+T from remote tabs page in tab grid.

|-openNewTabInPage:focusOmnibox:| was a no-op if the page was the
remote tabs  page, but TabGridCoordinator's |-showActiveTabInPage:
focusOmnibox:| used a NOTREACH() when called with the remote tabs page.
This CL updates them both to use NOTREACH(), and short-circuits from the
command+T handler if the page is the remote tabs page.

Bug:  869820 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib394cc37a2b70420d3611e1440ad2b628a27c0a6
Reviewed-on: https://chromium-review.googlesource.com/1159087
Reviewed-by: edchin <edchin@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579901}(cherry picked from commit 512743d1acf7b809250f4aae884b30428037446f)
Reviewed-on: https://chromium-review.googlesource.com/1166123
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#477}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/ff004551655b0ba11c28c884bdcb5047aa1bfda4/ios/chrome/browser/ui/tab_grid/tab_grid_view_controller.mm

Verified in 69.0.3497.31 Beta,  iPad Air  iOS11.4
Looks good.

Sign in to add a comment