Go back does not work for interstitials that are in the middle of navigation stack |
|||||
Issue descriptionApp Version (from "Chrome Settings > About Chrome"): TOT iOS Version: iOS 10.2 Device: iPhone SE Steps to reproduce: 1.) Turn on Airplane Mode 2.) Go to expired.badssl.com 3.) Go to google.com 4.) Turn off Airplane Mode 5.) Go Back 6.) Go Back Observed behavior: google.com is loaded (app went forward) Expected behavior: NTP is loaded (app should go back)
,
Dec 27 2016
GetIndexForOffset fix is here: https://codereview.chromium.org/2600263002/ The bug is still reproducible even after GetIndexForOffset fix.
,
Dec 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8f276d123db9ccd82569be7cb8658fde3999ae59 commit 8f276d123db9ccd82569be7cb8658fde3999ae59 Author: eugenebut <eugenebut@chromium.org> Date: Tue Dec 27 23:44:44 2016 [ios] Fixed GetIndexForOffset for pending transient items. GetIndexForOffset() should decrease resulting index by 1 for pending transient items to allow going back from interstitials that were added to the middle of the stack. BUG= 677190 Review-Url: https://codereview.chromium.org/2600263002 Cr-Commit-Position: refs/heads/master@{#440808} [modify] https://crrev.com/8f276d123db9ccd82569be7cb8658fde3999ae59/ios/web/navigation/navigation_manager_impl.mm [modify] https://crrev.com/8f276d123db9ccd82569be7cb8658fde3999ae59/ios/web/navigation/navigation_manager_impl_unittest.mm
,
Feb 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2517f141263ee8913024a3a96cd909423ad7214 commit b2517f141263ee8913024a3a96cd909423ad7214 Author: eugenebut <eugenebut@chromium.org> Date: Tue Feb 14 03:21:43 2017 Fixed navigation for displayed SSL interstitials. Navigation did not work for interstitials for 2 reasons which are fixed in this CL: 1.) |WebInterstitialImpl::DontProceed| should not reload. Reloading stops pending navigation and reloads last committed item, which is not always correct target item. On other platforms InterstitialPageImpl::DontProceed does not reload. 2.) |clearTransientContentView| removes pending item, so it should be called before pending navigation is initiated. So next |clearTransientContentView| call will be no-op. This CL changes the behavior, so when user taps "BACK TO SAFETY" the page is not reloaded. This is fine, but in order to update omnibox URL, tab should subscribe to webStateDidDismissInterstitial: callback and update the toolbar. BUG= 677193 , 677190 , 688009 Review-Url: https://codereview.chromium.org/2687343003 Cr-Commit-Position: refs/heads/master@{#450220} [modify] https://crrev.com/b2517f141263ee8913024a3a96cd909423ad7214/ios/chrome/browser/tabs/tab.mm [modify] https://crrev.com/b2517f141263ee8913024a3a96cd909423ad7214/ios/web/interstitials/web_interstitial_impl.mm [modify] https://crrev.com/b2517f141263ee8913024a3a96cd909423ad7214/ios/web/web_state/ui/crw_web_controller.mm
,
Feb 14 2017
,
Feb 15 2017
NTP is loaded correctly after step#6 form original steps to reproduce. Verified on M58.0.3013.0 canary Device: iPhone7, iOS: 10.3
,
Feb 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9674693c37dc34ad55e2f21f19050ed2938d6d1c commit 9674693c37dc34ad55e2f21f19050ed2938d6d1c Author: eugenebut <eugenebut@chromium.org> Date: Thu Feb 16 01:36:27 2017 Fixed navigation for displayed SSL interstitials. Navigation did not work for interstitials for 2 reasons which are fixed in this CL: 1.) |WebInterstitialImpl::DontProceed| should not reload. Reloading stops pending navigation and reloads last committed item, which is not always correct target item. On other platforms InterstitialPageImpl::DontProceed does not reload. 2.) |clearTransientContentView| removes pending item, so it should be called before pending navigation is initiated. So next |clearTransientContentView| call will be no-op. This CL changes the behavior, so when user taps "BACK TO SAFETY" the page is not reloaded. This is fine, but in order to update omnibox URL, tab should subscribe to webStateDidDismissInterstitial: callback and update the toolbar. BUG= 677193 , 677190 , 688009 TBR=kkhorimoto@chromium.org NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2687343003 Cr-Commit-Position: refs/heads/master@{#450220} (cherry picked from commit b2517f141263ee8913024a3a96cd909423ad7214) Review-Url: https://codereview.chromium.org/2694623013 Cr-Commit-Position: refs/branch-heads/2987@{#537} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/9674693c37dc34ad55e2f21f19050ed2938d6d1c/ios/chrome/browser/tabs/tab.mm [modify] https://crrev.com/9674693c37dc34ad55e2f21f19050ed2938d6d1c/ios/web/interstitials/web_interstitial_impl.mm [modify] https://crrev.com/9674693c37dc34ad55e2f21f19050ed2938d6d1c/ios/web/web_state/ui/crw_web_controller.mm
,
Feb 22 2017
Verified on chrome beta version 57.0.2987.72 on iPhone 7 plus with iOS 10.2.1, iPad Air 10.2.1 following steps mentioned in comment #0. NTP is displayed after step #6. Looks good. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by eugene...@chromium.org
, Dec 27 2016