New issue
Advanced search Search tips
Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug-Regression

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 94806: Editing link-navigated intranet URLs should result in navigations, not searches

Reported by pkasting@chromium.org, Aug 30 2011 Project Member

Issue description

This is an edge case, but I should avoid shipping it to M15.

If you navigate to a new intranet host without typing it, then hitting enter in the address bar will search instead of reload.  This is because the HUP is failing to find this as a past typed hostname.  The best fix here is probably to make schemes not be stripped in this case.  Unfortunately we can't call Parse() from net_util.cc so I'll probably need to write a wrapper.
 

Comment 1 by pkasting@chromium.org, Aug 30 2011

Brett's comment is that if possible we should hoist FormatUrl() out of net and up into chrome.

Comment 2 by pkasting@chromium.org, Sep 22 2011

 Issue 97648  has been merged into this issue.

Comment 3 by pkasting@chromium.org, Sep 29 2011

 Issue 98494  has been merged into this issue.

Comment 4 by ligim...@chromium.org, Oct 6 2011

 Issue 98256  has been merged into this issue.

Comment 5 by pkasting@chromium.org, Oct 11 2011

Cc: mihaip@chromium.org
 Issue 99894  has been merged into this issue.

Comment 6 by pkasting@chromium.org, Oct 12 2011

Another possible fix:

Perhaps if we navigate to an intranet URL, and you haven't typed that hostname before, we should add a synthetic "typed" navigation to the bare hostname to force future navigations on that URL to "just work".

This may be more complex to implement, and I don't know what effect it will have on the history page.  OTOH it is a broader fix so we'll annoy users less.

Comment 7 by jshute@google.com, Oct 12 2011

Can you just have a heuristic that anything that looks like host:port, or has slashes and no spaces, is much more likely to be a url than a string that should be searched in google?

Comment 8 by pkasting@chromium.org, Oct 12 2011

That's  bug 99131 , not this bug.

And no, "slashes and no spaces" alone turns out not to be a good heuristic as there are lots of searches like "ps/2" that run afoul of that one.

Comment 9 by pkasting@chromium.org, Oct 13 2011

Status: Started
Summary: Editing link-navigated intranet URLs should result in navigations, not searches
Fix is now up for review on http://codereview.chromium.org/8286011/ .  I ended up implementing a variant of comment 6 that just changes the navigation type to TYPED in these cases, which was relatively easy to implement and should not only fix this bug but also make it more likely that the omnibox will do "the right thing" with future attempts to type said URLs.

Comment 10 by pkasting@chromium.org, Oct 14 2011

Labels: Merge-Requested
Fixed on trunk in r105565.

Comment 11 by bugdroid1@chromium.org, Oct 14 2011

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=105565

------------------------------------------------------------------------
r105565 | pkasting@chromium.org | Fri Oct 14 13:42:49 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/history/url_database.cc?r1=105565&r2=105564&pathrev=105565
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/autocomplete/history_url_provider.cc?r1=105565&r2=105564&pathrev=105565
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/history/history_backend.cc?r1=105565&r2=105564&pathrev=105565
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/history/url_database.h?r1=105565&r2=105564&pathrev=105565
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/history/history_unittest.cc?r1=105565&r2=105564&pathrev=105565

When users navigate to an intranet host that has not been previously typed, treat it as a typed navigation.

This fixes two different problems:
(1) Due to scheme-stripping, a user who navigated to e.g. "host/path" via a link or bookmark, for a not-previously-typed host, would erroneously get a search page instead of a navigation when reloading via hitting enter or when editing the path to be something else.  We could instead fix this by being more conservative about scheme-stripping, but that is uglier UI, a bigger change, and doesn't solve problem (2).
(2) Users who navigated to intranet hosts (with or without a path or other URL components) via link clicks/bookmarks/etc. would later try to type them and be frustrated that the omnibox would do a search and then show the accidental search infobar.  This change makes the omnibox more aggressive about learning that such hostnames are valid intranet hosts even before users type them, if they've at least visited them.

There are four issues here:
(1) Because in incognito mode we never add navigations to any history DBs, users there won't see the above benefits.  In fact, users there will struggle to use (new) intranet hosts in general.  I filed bug 100271 about this.
(2) The omnibox will now be more likely to inline-autocomplete intranet URLs you haven't actually typed before.  After thinking for a while I don't think this will have a large effect either way especially as intranet URLs are often short (and thus it's less likely users will try to search or navigate to a pure prefix of one).
(3) Heavy intranet users will have a larger in-memory DB.  I don't think this effect will be dramatic since it's just one URL per unique hostname, and there's no real way around this anyway.
(4) The additional checking in AddPage() could make navigation slower.  I don't think this ought to happen because most navigations will fail the RCDS check for "no known TLD" (which ought to be fast, as that code is called by things like cookie management), so they'll never even check the database.  If this turns out to be wrong, then we'll need to fall back on something like the solution in bug 100271.

BUG= 94806 
TEST=On a clean profile, add a bookmark for some intranet host (without navigating there), with or without a path.  Click on the bookmark.  Then try to modify the path and check that the omnibox still wants to navigate, not search.
Review URL: http://codereview.chromium.org/8286011
------------------------------------------------------------------------

Comment 12 by kareng@google.com, Oct 17 2011

Labels: -Merge-Requested Merge-Approved

Comment 13 by pkasting@chromium.org, Oct 17 2011

Status: Fixed
Merged to 874 in r105892.

Comment 14 by bugdroid1@chromium.org, Oct 17 2011

Project Member
Labels: -merge-approved merge-merged-874
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=105892

------------------------------------------------------------------------
r105892 | pkasting@chromium.org | Mon Oct 17 13:19:31 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/874/src/chrome/browser/history/url_database.cc?r1=105892&r2=105891&pathrev=105892
 M http://src.chromium.org/viewvc/chrome/branches/874/src/chrome/browser/autocomplete/history_url_provider.cc?r1=105892&r2=105891&pathrev=105892
 M http://src.chromium.org/viewvc/chrome/branches/874/src/chrome/browser/history/history_backend.cc?r1=105892&r2=105891&pathrev=105892
 M http://src.chromium.org/viewvc/chrome/branches/874/src/chrome/browser/history/url_database.h?r1=105892&r2=105891&pathrev=105892
 M http://src.chromium.org/viewvc/chrome/branches/874/src/chrome/browser/history/history_unittest.cc?r1=105892&r2=105891&pathrev=105892

Merge 105565 - When users navigate to an intranet host that has not been previously typed, treat it as a typed navigation.

This fixes two different problems:
(1) Due to scheme-stripping, a user who navigated to e.g. "host/path" via a link or bookmark, for a not-previously-typed host, would erroneously get a search page instead of a navigation when reloading via hitting enter or when editing the path to be something else.  We could instead fix this by being more conservative about scheme-stripping, but that is uglier UI, a bigger change, and doesn't solve problem (2).
(2) Users who navigated to intranet hosts (with or without a path or other URL components) via link clicks/bookmarks/etc. would later try to type them and be frustrated that the omnibox would do a search and then show the accidental search infobar.  This change makes the omnibox more aggressive about learning that such hostnames are valid intranet hosts even before users type them, if they've at least visited them.

There are four issues here:
(1) Because in incognito mode we never add navigations to any history DBs, users there won't see the above benefits.  In fact, users there will struggle to use (new) intranet hosts in general.  I filed bug 100271 about this.
(2) The omnibox will now be more likely to inline-autocomplete intranet URLs you haven't actually typed before.  After thinking for a while I don't think this will have a large effect either way especially as intranet URLs are often short (and thus it's less likely users will try to search or navigate to a pure prefix of one).
(3) Heavy intranet users will have a larger in-memory DB.  I don't think this effect will be dramatic since it's just one URL per unique hostname, and there's no real way around this anyway.
(4) The additional checking in AddPage() could make navigation slower.  I don't think this ought to happen because most navigations will fail the RCDS check for "no known TLD" (which ought to be fast, as that code is called by things like cookie management), so they'll never even check the database.  If this turns out to be wrong, then we'll need to fall back on something like the solution in bug 100271.

BUG= 94806 
TEST=On a clean profile, add a bookmark for some intranet host (without navigating there), with or without a path.  Click on the bookmark.  Then try to modify the path and check that the omnibox still wants to navigate, not search.
Review URL: http://codereview.chromium.org/8286011

TBR=pkasting@chromium.org
Review URL: http://codereview.chromium.org/8316017
------------------------------------------------------------------------

Comment 15 by bugdroid1@chromium.org, Oct 17 2011

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=105906

------------------------------------------------------------------------
r105906 | pkasting@chromium.org | Mon Oct 17 13:48:00 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/874/src/chrome/browser/history/history_unittest.cc?r1=105906&r2=105905&pathrev=105906

Fix typo in merge conflict resolution.

BUG= 94806 

------------------------------------------------------------------------

Comment 16 by bugdroid1@chromium.org, Oct 13 2012

Project Member
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 17 by bugdroid1@chromium.org, Mar 9 2013

Project Member
Labels: -Type-Regression -Area-UI -Mstone-15 -Feature-Omnibox Type-Bug-Regression Cr-UI Cr-UI-Browser-Omnibox M-15

Comment 18 by bugdroid1@chromium.org, Mar 13 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment