New issue
Advanced search Search tips

Issue 913052 link

Starred by 1 user
Status: Fixed
Owner:
eugene...@chromium.org
Closed: Jan 2
Cc:
ios-bugs@chromium.org
ajuma@chromium.org
kkhorimoto@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug

Blocking:
issue 734150



Sign in to add a comment

has_user_gesture should be true for back/forward navigation in slim nav

Project Member Reported by danyao@google.com, Dec 7 
WKBasedNavigationManagerImpl::FinishGoToIndex() should propagate the |has_user_gesture| value to the NavigationContext created as a result of the back/forward navigation.

Comment 1 by danyao@google.com, Dec 7 

This is hard to fix today because NavigationContext for back/forward navigation is created asynchronously in the |webView:decidePolicyForNavigationAction:| callback in CRWWebController. It'll be easier to fix once WKNavigationDelegate logic moves into navigation manager.

Comment 2 by eugene...@chromium.org, Dec 7 

Can we investigate if this bug has serious effect on the app?

Comment 3 by danyao@google.com, Dec 7 

Sure. Do you remember why we introduced has_user_gesture?

Comment 4 by eugene...@chromium.org, Dec 7 

I think this flag is mostly used for blocking unwanted actions from web pages. "false" means more secure, but can break compatibility with existing web sites.
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 19 

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

commit 8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4
Author: Eugene But <eugenebut@google.com>
Date: Wed Dec 19 21:31:57 2018

Fix WebStateObserverTest.UserInitiatedHashChangeNavigation for slim nav.

Create navigation content at the moment when navigation to back forward
list is initiated. This allows to associate navigation context with
correct WKNavigation object. This makes PendingBackForwardContext
concept obsolete.

Bug:  913052 ,  851119 
Change-Id: I1e30f5feee7bfc82a4bbc8f46a459d7cd3a43a64
Reviewed-on: https://chromium-review.googlesource.com/c/1370374
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617954}
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/legacy_navigation_manager_impl.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/legacy_navigation_manager_impl.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/navigation_manager_delegate.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/wk_based_navigation_manager_impl.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/test/fakes/fake_navigation_manager_delegate.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/test/fakes/fake_navigation_manager_delegate.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/ui/crw_web_controller_unittest.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/ui/crw_web_view_navigation_proxy.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/ui/crw_wk_navigation_states.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4/ios/web/web_state/web_state_observer_inttest.mm
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 20 

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

commit fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b
Author: CJ DiMeglio <lethalantidote@chromium.org>
Date: Thu Dec 20 00:52:08 2018

Revert "Fix WebStateObserverTest.UserInitiatedHashChangeNavigation for slim nav."

This reverts commit 8f78fecf6ba90f38fc60c6cb41fb6c98a20354a4.

Reason for revert: Believed to be the cause of https://bugs.chromium.org/p/chromium/issues/detail?id=916829

Original change's description:
> Fix WebStateObserverTest.UserInitiatedHashChangeNavigation for slim nav.
> 
> Create navigation content at the moment when navigation to back forward
> list is initiated. This allows to associate navigation context with
> correct WKNavigation object. This makes PendingBackForwardContext
> concept obsolete.
> 
> Bug:  913052 ,  851119 
> Change-Id: I1e30f5feee7bfc82a4bbc8f46a459d7cd3a43a64
> Reviewed-on: https://chromium-review.googlesource.com/c/1370374
> Reviewed-by: Danyao Wang <danyao@chromium.org>
> Commit-Queue: Eugene But <eugenebut@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#617954}

TBR=eugenebut@chromium.org,danyao@chromium.org

Change-Id: I813f4231c4cf1913e695dc94e2c25088e1f6fb59
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  913052 ,  851119 
Reviewed-on: https://chromium-review.googlesource.com/c/1385481
Reviewed-by: CJ DiMeglio <lethalantidote@chromium.org>
Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618048}
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/legacy_navigation_manager_impl.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/legacy_navigation_manager_impl.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/navigation_manager_delegate.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/wk_based_navigation_manager_impl.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/test/fakes/fake_navigation_manager_delegate.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/test/fakes/fake_navigation_manager_delegate.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/ui/crw_web_controller_unittest.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/ui/crw_web_view_navigation_proxy.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/ui/crw_wk_navigation_states.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/fdc3adbb3c6b19359dbc8d8aa0bc9a7a33448a7b/ios/web/web_state/web_state_observer_inttest.mm
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 21 

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

commit 08d9fa1e884873016c608ddb604580b572cc19cb
Author: Eugene But <eugenebut@chromium.org>
Date: Fri Dec 21 15:23:07 2018

Reland Fix WebStateObserverTest.UserInitiatedHashChangeNavigation for slim nav.

Create navigation content at the moment when navigation to back forward
list is initiated. This allows to associate navigation context with
correct WKNavigation object. This makes PendingBackForwardContext
concept obsolete.

Original CL: https://chromium-review.googlesource.com/c/1370374

Bug:  913052 ,  851119 
Change-Id: I05678a686a50e3c1c0e1948225586b47dec202b3
Reviewed-on: https://chromium-review.googlesource.com/c/1385490
Reviewed-by: Danyao Wang <danyao@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618524}
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/legacy_navigation_manager_impl.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/legacy_navigation_manager_impl.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/navigation_manager_delegate.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/navigation_manager_impl.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/navigation_manager_impl_unittest.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/wk_based_navigation_manager_impl.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/wk_based_navigation_manager_impl.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/test/fakes/fake_navigation_manager_delegate.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/test/fakes/fake_navigation_manager_delegate.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/ui/crw_web_controller.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/ui/crw_web_controller.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/ui/crw_web_controller_unittest.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/ui/crw_web_view_navigation_proxy.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/ui/crw_wk_navigation_states.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/08d9fa1e884873016c608ddb604580b572cc19cb/ios/web/web_state/web_state_observer_inttest.mm

Comment 8 by eugene...@chromium.org, Jan 2

Cc: -eugene...@chromium.org
Owner: eugene...@chromium.org
Status: Fixed (was: Available)
►

Sign in to add a comment