Remove canGoBack/canGoForward checks from chromeExecuteCommand: |
||||
Issue descriptionThere are 2 equally valid options: 1.) Change NavigationManager's and NavigationController's GoBack() and GoForward() so they check CanGoBack and CanGoForward. 2.) Change KeyCommandsProvider so it does not send navigation messages if navigation is not possible Both options have advantages and disadvantages. #1 will not crash where no-op is preferable (like keyboard comment), but can mask potential bugs (like enabled back button, when no back entry exists). #2 will crash, but will expose UI bugs very efficiently. If creis@ is fine with changing NavigationController (https://bugs.chromium.org/p/chromium/issues/detail?id=676727#c3) then we should go with option #1. Otherwise we should go with option #2 to be aligned with content behavior.
,
Jan 13 2017
The KeyCommandsProvider only sends the IDC_BACK command to the BVC, and the BVC already makes the canGoBack/Forward checks thanks to you :) https://codereview.chromium.org/2605713002 So I only remove the TODOs here: https://codereview.chromium.org/2630733002
,
Jan 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9685e84627da279a93729b53ac3151b67d7eae46 commit 9685e84627da279a93729b53ac3151b67d7eae46 Author: lpromero <lpromero@chromium.org> Date: Fri Jan 13 16:34:35 2017 Remove obsolete TODO BUG= 677160 R=eugenebut@chromium.org,marq@chromium.org Review-Url: https://codereview.chromium.org/2630733002 Cr-Commit-Position: refs/heads/master@{#443574} [modify] https://crrev.com/9685e84627da279a93729b53ac3151b67d7eae46/ios/chrome/browser/ui/browser_view_controller.mm
,
Jan 13 2017
Uploaded https://codereview.chromium.org/2631733002.
,
Jan 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47ea8860bd01ee9a9498a893ff9eeec2618fa75e commit 47ea8860bd01ee9a9498a893ff9eeec2618fa75e Author: lpromero <lpromero@chromium.org> Date: Fri Jan 13 17:51:06 2017 Avoid sending IDC_BACK/FORWARD when back/forward is not possible This moves down logic to the KeyCommandsProvider, that registers key commands handlers that check if navigation is possible before sneding IDC_BACK/FORWARD Chrome commands. BUG= 677160 R=eugenebug@chromium.org,marq@chromium.org Review-Url: https://codereview.chromium.org/2631733002 Cr-Commit-Position: refs/heads/master@{#443603} [modify] https://crrev.com/47ea8860bd01ee9a9498a893ff9eeec2618fa75e/ios/chrome/browser/ui/browser_view_controller.mm [modify] https://crrev.com/47ea8860bd01ee9a9498a893ff9eeec2618fa75e/ios/chrome/browser/ui/key_commands_provider.h [modify] https://crrev.com/47ea8860bd01ee9a9498a893ff9eeec2618fa75e/ios/chrome/browser/ui/key_commands_provider.mm
,
Jan 13 2017
,
Jan 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7658fd0442cce20cb16ada469d812a94655270a commit d7658fd0442cce20cb16ada469d812a94655270a Author: eugenebut <eugenebut@chromium.org> Date: Sat Jan 14 00:00:44 2017 [ios] Made goToItemAtIndex: safe for release builds. Changed -[CRWWebController goToItemAtIndex:] to make no-op on release if index is out of bound. This fully mirrors NavigationControllerImpl::GoToIndex, which calls NOTREACHED() and early returns for out of bounds indices. BUG= 677160 Review-Url: https://codereview.chromium.org/2628133003 Cr-Commit-Position: refs/heads/master@{#443732} [modify] https://crrev.com/d7658fd0442cce20cb16ada469d812a94655270a/ios/web/web_state/ui/crw_web_controller.mm |
||||
►
Sign in to add a comment |
||||
Comment 1 by eugene...@chromium.org
, Jan 12 2017Owner: lpromero@chromium.org