-
Emily Stark authored
In the hide-on-interaction field trial, we were previously handling navigations only in DidFinishNavigation(). That is, when a navigation began, we didn’t change the elision state until it commited in DidFinishNavigation(). This worked well for renderer-initiated navigations, where the visible URL doesn’t update until the navigation commits. But it didn’t work for browser-initiated navigations, where the URL would update at the very beginning of the navigation and inherit the elision state of the previous navigation until DidFinishNavigation (see screencast in crbug.com/1111665). The fix for this is rather tricky and not perfect. For a browser-initiated navigation, the order of events is as follows: - URL is updated (triggering EmphasizeURLComponents) - DidStartNavigation - DidFinishNavigation Because the URL is updated at the first step, we need to set the elision state in EmphasizeURLComponents. Even setting in DidStartNavigation would be too late, as we could still get a flash of the new URL with the previous navigation’s elision applied to it. The problem, though, is that we don’t know if we should elide the URL until the navigation finishes, because the elision logic depends on whether the navigation was same-document and if it changed the visible URL’s path. (In particular, for same-document navigations that only change the fragment, we ignore those and don’t have them change the elision state at all.) Therefore, in EmphasizeURLComponents(), we don’t know for sure whether we should leave the URL elided if it already is elided, or if we should unelide and reset to the on-page-load state. I wasn’t able to think up a perfect solution for this, but the best I can come up with is to make our best guess how to elide in EmphasizeURLComponents(), and then fix it up as we get more information in DidStartNavigation() and DidFinishNavigation(). Specifically, EmphasizeURLComponents() elides to the simplified domain if the URL had already been elided, and shows the full URL otherwise. DidStartNavigation() then fully resets to the on-page-load state for cross-document navigations, and DidFinishNavigation() fully resets to the on-page-load state for same-document navigations that changed the path (which we don't know until the navigation commits). There remains a weirdness where a browser-initiated navigation will initially briefly show the simplified domain of the destination before updating to the full URL in DidStartNavigation(), but I think this is far better than briefly showing the destination URL truncated where the previous page’s URL was truncated. Bug: 1111665, 1109778 Change-Id: Ia1528254b8203dee93bd0964666b76d4b15bfd46 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2360803 Commit-Queue: Emily Stark <estark@chromium.org> Reviewed-by:
Scott Violet <sky@chromium.org> Reviewed-by:
Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#799914}
78764069