New issue
Advanced search Search tips

Issue 595524 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: 2019-01-08
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Allow Omnibox to Match in Port Number

Reported by zhifangy...@gmail.com, Mar 17 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36

Steps to reproduce the problem:
1. visit this url http://101.200.175.188:9880/account?id=2fb4ba81e9d211e5870300163e00371a
2. open new tab
3. type 9880

What is the expected behavior?
the url should show up in auto completion list

What went wrong?
the url now showing up in auto completion list

Did this work before? No 

Chrome version: 49.0.2623.87  Channel: stable
OS Version: OS X 10.11.3
Flash Version: Shockwave Flash 21.0 r0

currently, change step3 to: type 101, then the url will show up in completion list, but it is not good enough, 9880 is more identifiable. Why can't I search any part of the url??
 
sorry, typo in "What went wrong?", it should be "the url NOT showing up in auto completion list".

Comment 2 by shrike@chromium.org, Mar 19 2016

Components: -UI UI>Browser>Omnibox Internals
Labels: OS-Linux OS-Windows
Status: Untriaged (was: Unconfirmed)
The only thing that works is typing 101 - i.e. typing 200 or any other part of the URL does not cause the URL to show up in the completion list. So I think the intent is to match the start of the IP address rather than any portion of it.

Components: -Internals
Owner: mpear...@chromium.org
We're supposed to do mid-string matches in the hostname, so 200 should work.

As for searching the port, I suspect we've just never indexed that, because it's so rarely important.  But by that same token indexing it and searching in it probably wouldn't be hugely detrimental, it would just rarely have an effect.

Mark, thoughts?
It's very important sometime!
I am a developer and I use different ports for different apps in dev server, it will be very convenient if I can just type the port and hit enter to visit different apps.
Chrome has been a very good tool for developers, please make it better.

Cc: pkasting@chromium.org
Labels: -OS-Linux -OS-Windows -Pri-2 -OS-Mac OS-All Pri-3
Status: Assigned (was: Untriaged)
I finally looked into this.  Chrome indexes the full URL.  However, the part of the system that score/returns URL gives no credit to matches in what Chrome thinks is the top-level domain part of the URL (e.g., .com, .gov).  It looks like this code is inadvertently preventing matches in the port section as well.

This is an easy fix.  I don't think we need to test it given how few URLs people run into nowadays have port numbers explicitly in them.

pkasting@: you have an instinct for these type of things.  Do you think port numbers should score as highly as the rest of the hostname, or more like a path component, or more like a ?cgi fragment?

Not hostname.  One of the other two I think.  I'd probably start with CGI fragment (which is the lowest-scoring, right?) and see what that gets us.
Status: Started (was: Assigned)
Owner: ----
Status: Available (was: Started)
Marking as available.  I will not have time to fix this correctly.  Also see bug 448659, which covers a similar situation that could be fixed by the more general solution of using the proper URL segmentation mechanism.

Summary: Allow Omnibox to Match in Port Number (was: Improve fuzzy search in address bar for history urls)
[clarifying summary]
For reference, the way to fix this properly is expressed in pkasting's comment on
https://codereview.chromium.org/2355053003/
(which was my simple fix).

The central point is
>>>
It seems like rewriting [scored_history_match GetTopicality()] to use the standard machinery for parsing URLs can locate things like the hostname boundaries, port, CGI params, etc. with complete accuracy, in noticeably less code, and I'm not convinced will lose performance.   I think it is worth taking the time to do.
>>>

Owner: tommycli@chromium.org
Status: Started (was: Available)
Gimme
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 10 2017

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

commit efcd3a7e1ec0e95cec770ff5340cf1ba6b430385
Author: Tommy C. Li <tommycli@chromium.org>
Date: Mon Jul 10 23:42:15 2017

Omnibox: Refactor ScoredHistoryMatch URL parsing

Previously, we determined which part of the URL the match was in by
by searching the formatted URL string (in string16 form) for delimiters.

After this refactor, we do this by using the "official" GURL component
offsets adjusted for URL formatting.

This is good both as a refactor, and also to lay the groundwork for
adding |match_in_subdomain| and |match_in_path| flags.

This CL doesn't actually add the above flags, and is intended to
provide identical functionality as before (which is why no tests
changed).

Bug:  732582 , 595524, 448659
Change-Id: I133c2ecb462597941b7284fd88f99e55f341f6b4
Reviewed-on: https://chromium-review.googlesource.com/564300
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485446}
[modify] https://crrev.com/efcd3a7e1ec0e95cec770ff5340cf1ba6b430385/components/omnibox/browser/scored_history_match.cc
[modify] https://crrev.com/efcd3a7e1ec0e95cec770ff5340cf1ba6b430385/components/omnibox/browser/scored_history_match.h
[modify] https://crrev.com/efcd3a7e1ec0e95cec770ff5340cf1ba6b430385/components/omnibox/browser/scored_history_match_unittest.cc

Nice!  For the record, I would like to be at least CCed or the reviewer on code used in ranking, especially code that I wrote. :-)

tommycli@: are you planning to finish the rest of this bug?  It should be easy, and as I wrote in comment #5, I don't think we need to test it as a field trial given how few URLs people run into nowadays have port numbers explicitly in them.
Hey mpearson: I am indeed planning on taking this to the end.

I will be marking you as the reviewer on that CL. I wasn't trying to bypass you -- I didn't realize you were the author of the ranking code. ;) Thanks!
Labels: Hotlist-OmniboxRanking
mpearson@: Is it worth adding a per-file OWNERS entry for things you particularly care about?

tommycli@: Is this still on your plate?
re c#16: Yes I'm still planning on tackling this eventually, and my work in the patch above is partially relevant.

If someone else has the motivation to investigate this before me though, by all means take it.
> mpearson@: Is it worth adding a per-file OWNERS entry for
> things you particularly care about?
I think that's too heavy-handed.  I think stating a preference and letting people use their judgment about when it's appropriate is fine.
Cc: tommycli@chromium.org
Owner: ----
Status: Available (was: Started)
Marking as Available so someone else can tackle as wanted. It's not an immediate TODO for me.

Next person... looking at the rfind('.', ...) in the above CL is the right place to start to make this work correctly for IP addresses...
FWIW, I strongly encourage mpearson being listed as an OWNER in the directory list alongside other people, or at least per-file.

OWNERS lists aren't heavy-handed so much as informational.  When people come in to an area, they tell you who you can send reviews to and who you can contact to learn more.  I don't view being listed in one as a way to control things but as a recognition of "I know things here and I'm happy to help".  In that sense Mark would be eminently appropriate.
Agree with Peter. I'm thinking of adding myself to the OWNER files on some of them, but I feel that Mark should go in way before me.
I don't understand.  I am already an OWNER of the directory.  I suppose I could add a restriction to the directory so I'm the sole owner of things like scored_history_match*, in_memory_url_index_*, and maybe a few others but that seems heavy-handed.  And I feel like putting a comment in the OWNERS files about preferred reviewers per-file seems not likely to do much.  It's not like we're getting many reviews like the one that brought up this issue from outside the team, and people inside the team aren't likely to bother reading the owners files for comments.  Besides, don't most people pick reviewers by looking at who seems to be familiar with the relevant code?  I don't see how a comment adds value here.
My comment was based on the assumption that you were not listed as an OWNER in the overall list.  If you are, great!

It's sufficient in my book if other OWNERS know "Mark is the best reviewer for this area".
Project Member

Comment 24 by sheriffbot@chromium.org, Aug 9

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
NextAction: 2019-01-08
Status: Available (was: Untriaged)
Still valid, still low priority.
The NextAction date has arrived: 2019-01-08

Sign in to add a comment