chrome://newtab displayed instead of "New Tab" on history popup menu |
||||||||||||
Issue descriptionWith SlimNavigationManager enabled. What steps will reproduce the problem? (1) Open a New Tab (2) Navigate to a web page (3) Relaunch the app (4) Long Press on the back arrow What is the expected result? The popup menu should contain "New Tab". What happens instead? The history entry is labelled "chrome://newtab". The title is displayed using navigationItem->GetTitleForDisplay(). https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/popup_menu/popup_menu_mediator.mm?type=cs&q=GetTitleForDisplay&sq=package:chromium&g=0&l=563
,
Sep 14
Issue 836087 has been merged into this issue.
,
Oct 23
,
Nov 2
,
Nov 2
,
Nov 19
,
Nov 19
,
Nov 19
,
Nov 19
,
Nov 21
Still an issue with Browser Container NTP enabled.
,
Nov 22
Additionally, this isn't specific to the NTP. If I navigate from the NTP to a bunch of wikipedia links, then background the app, and hold back, none of the correct titles appear.
,
Nov 23
danyao@ it looks like this fixes title restoration for urls N+1, but not the first URL:
diff --git a/ios/web/navigation/wk_based_navigation_manager_impl.mm b/ios/web/navigation/wk_based_navigation_manager_impl.mm
index c657e9138c78..c905ad9bfa81 100644
--- a/ios/web/navigation/wk_based_navigation_manager_impl.mm
+++ b/ios/web/navigation/wk_based_navigation_manager_impl.mm
@@ -740,6 +740,7 @@ WKBasedNavigationManagerImpl::WKWebViewCache::GetNavigationItemImplAtIndex(
nullptr /* use default rewriters only */);
new_item->SetTimestamp(
navigation_manager_->time_smoother_.GetSmoothedTime(base::Time::Now()));
+ new_item->SetTitle(base::SysNSStringToUTF16(wk_item.title));
const GURL& url = new_item->GetURL();
// If this navigation item has a restore_session.html URL, then it was created
// to restore session history and will redirect to the target URL encoded in
Is this fix reasonable? Does this belong in a different place?
It seems like the first URL starts off in WKBasedNavigation as a pending navigation, so it needs to be updated elsewhere.
,
Nov 23
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/594f66a8db0a96124dc39c54eb5dea590d1232a3 commit 594f66a8db0a96124dc39c54eb5dea590d1232a3 Author: Justin Cohen <justincohen@google.com> Date: Tue Nov 27 17:58:04 2018 [Nav Experiment] Correct navItem title usage. - Work around pushState changes that overwrite the navigation item title on pushState. The root cause of the bug is crbug.com/908173. - Correctly set navigation item titles on restore. Bug: 854174 , 872211 Change-Id: Ife407429d7f2723fc9dac6458bf81bdb1ae64ff8 Reviewed-on: https://chromium-review.googlesource.com/c/1349776 Commit-Queue: Justin Cohen <justincohen@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Danyao Wang <danyao@chromium.org> Cr-Commit-Position: refs/heads/master@{#611180} [modify] https://crrev.com/594f66a8db0a96124dc39c54eb5dea590d1232a3/ios/web/navigation/wk_based_navigation_manager_impl.mm [modify] https://crrev.com/594f66a8db0a96124dc39c54eb5dea590d1232a3/ios/web/navigation/wk_based_navigation_manager_impl_unittest.mm [modify] https://crrev.com/594f66a8db0a96124dc39c54eb5dea590d1232a3/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/594f66a8db0a96124dc39c54eb5dea590d1232a3/ios/web/web_state/web_state_unittest.mm
,
Nov 27
,
Dec 4
Verified in 73.0.3629.0 Canary in iPad Air(iOS 12.0.1) and iPhone 8plus(iOS 11.4.1) Followed the steps mentioned in comment#0 and comment#11. Looks good, 'New Tab' and correct title is displayed in history popup menu Link to video: https://drive.google.com/file/d/16xMUSpx9ud2Rxna-i9Hu_Bm4Ep8WE9BO/view?usp=sharing |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by danyao@chromium.org
, Sep 12