New issue
Advanced search Search tips

Issue 746641 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Selecting the Current URL from the Omnibox Causes a TYPED Navigation

Project Member Reported by mpear...@chromium.org, Jul 19 2017

Issue description

What steps will reproduce the problem?
(1) Focus on the omnibox.
(2) Press return to reload the page.

What is the expected result?
The page gets reloaded and marked in history as transition type RELOAD.

What happens instead?
The page gets reloaded and marked in history as transition type TYPED.


Tested on Linux.  Certainly likely to be a problem across all desktop platforms.  Pretty likely to be a problem on mobile platforms too.

 
When fixing this, attempt to also fix the case when the omnibox dropdown is open and the user selects the current URL (likely the top URL).  The dropdown can be open for a variety of reasons:
* user focused it and then pressed down
* user focused it and the omnibox decided to offer suggestions based on the current URL (enabled on desktop)
* user focused it on Android (most visited suggestions displayed by default)

Cc: treib@chromium.org
CC treib@ as an FYI, as I think they use typed visits to decide to decide on NTP tiles

@1: If we knew that e.g. the user pressed down-and-back-up and hit enter, I'd treat that as TYPED.  I might also treat "user pressed down arrow to open the dropdown, then hit enter" as TYPED.  The case I wouldn't treat as TYPED is "user didn't take any action to open the dropdown, but it was open anyway" (e.g. zerosuggest).

Comment 4 by treib@chromium.org, Jul 20 2017

Cc: mastiz@chromium.org
Thanks for the heads-up! +mastiz too.
FWIW though, I expect this case is rare enough that it won't have any substantial impact on the NTP tiles.
Labels: OS-Android OS-Chrome OS-iOS OS-Linux OS-Mac OS-Windows
Individually selecting operating systems because this will require different fixes in different places.

@treib comment #4:
> I expect this case is rare enough that it won't have any
> substantial impact on the NTP tiles.

Maybe.  Maybe not.  For reference, on Android about 4% of omnibox-mediated navigations (typed OR generated) are likely to be reloads.  If you restrict yourself to typed navigations, it's more like 7% of typed navigations are likely reloads.  If users select reloads more often on certain sites than others, this might be enough to alter the ranking.

Comment 7 by mastiz@chromium.org, Aug 10 2017

The server-side algorithm squashes repeated visits to the same URL within the same tab, so I believe this shouldn't be a real issue there.

For the client-side TopSites, we'd need to take a close look at the implementation, but I do suspect it could be affected.

Comment 8 by treib@chromium.org, Aug 11 2017

For the client-side TopSites, there needs to be a single TYPED (or AUTO_BOOKMARK) navigation for the page to be considered, but once there is one, we count all visits to the site towards its visit count, whatever their transition type - see HistoryBackend::UpdateSegments. (There is special handling for forward/back, but not for reload.) This is admittedly weird, but it's been like that since ~forever.
Good to hear this issue isn't causing problems in either server-side most visited.  It could be causing minor problems on client-side most visited if the initial TYPED navigation was due to a reload.  But I think that's unlikely; in all likelihood, this page will have gotten a typed visit somewhere, so reloads counting as typed versus not typed won't make a difference in how it's placed on the NTP.
I've been thinking more about how to fix this.  I think the most straightforward, fairly appropriate, way doesn't involve trying to track whether the user has explicitly edited the omnibox, unlike the discussions in comment #1 and comment #3.  Instead, I think if |destination_url| == |current_url|, we should treat the navigation as not-typed.  It doesn't matter what contortions the user did or did not do to end up selecting or pressing enter on a URL that corresponds to the current page.  If it's the current page, it's conceptually a reload.

This is consistent with the definitions of that transition type:
  // The user "reloaded" the page, either by hitting the reload button or by
  // hitting enter in the address bar.  NOTE: This is distinct from the
  // concept of whether a particular load uses "reload semantics" (i.e.
  // bypasses cached data).
https://cs.chromium.org/chromium/src/ui/base/page_transition_types.h?l=76-77

Comment 11 by holte@chromium.org, Aug 23 2017

Components: -Internals>Metrics
Labels: Hotlist-Metrics
Owner: mpear...@chromium.org
Status: Started (was: Available)
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 14 2017

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

commit 90388a48697cce7562029069c48312e9399a4f63
Author: Mark Pearson <mpearson@chromium.org>
Date: Thu Sep 14 00:54:19 2017

Omnibox - Reloads of the Current Page Should Not Count as Typed Navigations

Tested interactively by using about:omnibox "Show all details" to see the
typed and visit counts for a certain URL.  Did various actions and made
sure the counts only increased at appropriate times.  Test on both Linux
and Android.

Bug:  746641 
Change-Id: I067727912fa92cdc8b81689545ebd14a1ee17c92
Reviewed-on: https://chromium-review.googlesource.com/664067
Commit-Queue: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501818}
[modify] https://crrev.com/90388a48697cce7562029069c48312e9399a4f63/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java
[modify] https://crrev.com/90388a48697cce7562029069c48312e9399a4f63/components/omnibox/browser/omnibox_edit_model.cc

Status: Fixed (was: Started)
Labels: Needs-Feedback
Tested this issue on Windows 7, Ubuntu 14.04 and Mac OS using the latest Canary 63.0.3215.0 and Stable 61.0.3163.79 with the below steps.

1. Launched Chrome and navigated to chrome://omnibox.
2. In the textarea, entered www.starbucks.com and ticked on the 'Show all details' checkbox.
3. In a new tab, typed www.starbucks.com and hit on Enter key.
4. Went to chrome://omnibox page and clicked on Submit button. Could observe that the Typed and Visit count is appended to 1.
5. Repeated the steps and could observe that
- On selecting the website address -> hitting the Enter Key -> Typed and Visited Count is not appended.
- On typing the website address and selecting the address from the dropdown suggestions -> hit on enter key -> Typed and Visited count is appended.
- On typing the address and hit the right arrow key to complete it -> hit on the enter key -> Typed and Visited count is appended.

Retried the issue on M60 builds and could observe the same behavior as above.

Please find the attached screen-cast and confirm if the fix is working as intended and please help us understand what exact change is landed in the latest build.

Thanks..


746641.webm
10.8 MB View Download
Thanks for testing!  Unfortunately, you picked a bad example web site.  In this case, you're using about:omnibox to look at the typed counts for http://www.starbucks.com/.   
Yet you've navigated to httpS://www.starbucks.com/ (https).  Thus, when you try to reload that page, the counts for the former do not increase.  That's normal, and that the same as before my change.  And when you type in www.starbucks.com later in the video, you're navigating from httpS://www.starbucks.com/ to http://www.starbucks.com/ (because the default suggestion is http).  This isn't a reload.  Thus, you see the typed counts increase for http://www.starbucks.com/ both before my change and after my change.  My change only affects reloads.

Please re-test using a web site that does not do any redirection.

(You might not be able to reproduce the old bad behavior on all web sites; sometimes the old code worked fine.  But at least we should be able to verify that we don't see anything wrong.)

Sign in to add a comment