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

Issue 694971 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

⌘-g on NTP triggers DCHECK

Project Member Reported by jif@chromium.org, Feb 22 2017

Issue description

Pressing ⌘-g (which launches IDC_FIND_NEXT) leads to a DCHECK/crash.
Happens both on iPhone and iPad.

Unrelated: ⌘-g and ⌘-r do not appear in the list of shortcuts that is shown when long pressing ⌘.
 
On the unrelated: The list is not exhaustive by design. But it's weird that ⌘R is not present:
https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/key_commands_provider.mm?q=key_commands_pro&sq=package:chromium&dr=C&l=90
If there is a tab, it should appear. Consider filing a separate bug.

Can you paste the DCHECK, at least the first lines?


Comment 2 by jif@chromium.org, Feb 22 2017

in the console:
[0222/130802.861426:FATAL:crw_js_injection_manager.mm(55)] Check failed: [self hasBeenInjected].

the stack trace ( https://screenshot.googleplex.com/VrvbeViv1qn ):
#0	0x000000010eacf444 in base::debug::BreakDebugger() at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../base/debug/debugger_posix.cc:262
#1	0x000000010eb3f3a1 in logging::LogMessage::~LogMessage() at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../base/logging.cc:759
#2	0x000000010eb3bfd5 in logging::LogMessage::~LogMessage() at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../base/logging.cc:533
#3	0x000000010da80e08 in ::-[CRWJSInjectionManager inject]() at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../ios/web/web_state/js/crw_js_injection_manager.mm:55
#4	0x000000010e36138b in ::-[FindInPageController initFindInPage]() at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../ios/chrome/browser/find_in_page/find_in_page_controller.mm:139
#5	0x000000010e36293b in ::-[FindInPageController findNextStringInPageWithCompletionHandler:](ProceduralBlock) at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../ios/chrome/browser/find_in_page/find_in_page_controller.mm:239
#6	0x000000010e31b4bc in ::-[BrowserViewController chromeExecuteCommand:](id) at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../ios/chrome/browser/ui/browser_view_controller.mm:3846
#7	0x000000010e33a0c6 in ::__58-[KeyCommandsProvider keyCommandsForConsumer:editingText:]_block_invoke(NSInteger) at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../ios/chrome/browser/ui/key_commands_provider.mm:25
#8	0x000000010e33a9b4 in ::__58-[KeyCommandsProvider keyCommandsForConsumer:editingText:]_block_invoke.93() at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../ios/chrome/browser/ui/key_commands_provider.mm:126
#9	0x000000010dfa1faa in ::-[UIApplication(ChromeKeyCommandHandler) cr_handleKeyCommand:](UIKeyCommand *) at /Volumes/SSD-big/bling2/src/out/Debug-iphonesimulator/../../ios/chrome/browser/ui/keyboard/UIKeyCommand+Chrome.mm:30

Cc: rohitrao@chromium.org stkhapugin@chromium.org
Both Rohit and Stepan touched ios/chrome/browser/find_in_page recently. Could it be related?

Comment 4 Deleted

Comment 5 Deleted

Comment 6 by jif@chromium.org, Feb 22 2017

Just one tab

That one tab was showing the NTP

Occurs after a cold start. Never performed FIP before. Requires closing any opened infobar.

The find bar was closed.

Sometimes you may need to press Cmd-G multiple times.

/!\ Important: I'm hitting this in the simulator; I haven't tried on a real device.
Aha, thanks!

Find-in-page should be disabled on the NTP (it's greyed out in the tools menu), so we're invoking code that should never be run.

I probably broke this in https://codereview.chromium.org/2654433007, although looking at my changes it's entirely possible that this never worked.
Owner: ----
Status: Available (was: Assigned)
Project Member

Comment 9 by sheriffbot@chromium.org, May 3 2018

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Components: UI>Browser>NewTabPage
Owner: justincohen@chromium.org
Status: Assigned (was: Untriaged)
Justin, does this NTP bug still reproduce?
Cc: justincohen@chromium.org
Components: -UI>Browser>NewTabPage UI>Browser>FindInPage
Owner: stkhapugin@chromium.org
Yep, we shouldn't call -findNextStringInPage when on native pages that doens't support it.
Status: Started (was: Assigned)
Project Member

Comment 13 by bugdroid1@chromium.org, May 7 2018

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

commit 6daf1e26e6a517e3fcb8827a2c2d121c51a0372f
Author: stkhapugin@chromium.org <stkhapugin@chromium.org>
Date: Mon May 07 14:53:32 2018

[iOS] Disable find in page keyboard shortcuts on native content pages.

Disables keyboard shortcuts for FIP (cmd+F, cmd+G, cmd+shift+G) on NTP
and other pages where FIP is not available.

Bug:  694971 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7f32c55760a2ed976bac085994223afa50c3b976
Reviewed-on: https://chromium-review.googlesource.com/1046829
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556437}
[modify] https://crrev.com/6daf1e26e6a517e3fcb8827a2c2d121c51a0372f/ios/chrome/browser/ui/browser_view_controller.mm
[modify] https://crrev.com/6daf1e26e6a517e3fcb8827a2c2d121c51a0372f/ios/chrome/browser/ui/key_commands_provider.h
[modify] https://crrev.com/6daf1e26e6a517e3fcb8827a2c2d121c51a0372f/ios/chrome/browser/ui/key_commands_provider.mm
[modify] https://crrev.com/6daf1e26e6a517e3fcb8827a2c2d121c51a0372f/ios/chrome/browser/ui/key_commands_provider_unittest.mm

Status: Fixed (was: Started)

Sign in to add a comment