Omnibox UI Experiments: Hide parts of the URL in the suggestion dropdown |
|||||
Issue description
,
Jun 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd4ba279668fa6d606f622e023fb8923aa8923c1 commit fd4ba279668fa6d606f622e023fb8923aa8923c1 Author: tommycli <tommycli@chromium.org> Date: Wed Jun 14 00:05:39 2017 Omnibox UI Experiments: Add feature flags for 3 URL elision experiments. This CL contains just the feature flags. Implementation to follow shortly. BUG= 732582 Review-Url: https://codereview.chromium.org/2939563002 Cr-Commit-Position: refs/heads/master@{#479215} [modify] https://crrev.com/fd4ba279668fa6d606f622e023fb8923aa8923c1/chrome/browser/about_flags.cc [modify] https://crrev.com/fd4ba279668fa6d606f622e023fb8923aa8923c1/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/fd4ba279668fa6d606f622e023fb8923aa8923c1/chrome/browser/flag_descriptions.h [modify] https://crrev.com/fd4ba279668fa6d606f622e023fb8923aa8923c1/components/omnibox/browser/omnibox_field_trial.cc [modify] https://crrev.com/fd4ba279668fa6d606f622e023fb8923aa8923c1/components/omnibox/browser/omnibox_field_trial.h [modify] https://crrev.com/fd4ba279668fa6d606f622e023fb8923aa8923c1/tools/metrics/histograms/enums.xml
,
Jun 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/98195f33ec0f9d33f0099846674c2ebc1f5a7898 commit 98195f33ec0f9d33f0099846674c2ebc1f5a7898 Author: tommycli <tommycli@chromium.org> Date: Thu Jun 22 22:12:01 2017 Omnibox UI Experiments: Implement scheme-trimming for suggested URLs. Enabled by feature flag only. Only trims out HTTPs, since we already trim HTTP, and other schemes do not seem appropriate to trim out. BUG= 732582 TEST=unit and manual Review-Url: https://codereview.chromium.org/2940973002 Cr-Commit-Position: refs/heads/master@{#481687} [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/autocomplete_match.cc [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/autocomplete_match.h [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/autocomplete_match_unittest.cc [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/clipboard_url_provider.cc [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/history_quick_provider.cc [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/history_url_provider.cc [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/titled_url_match_utils.cc [modify] https://crrev.com/98195f33ec0f9d33f0099846674c2ebc1f5a7898/components/omnibox/browser/zero_suggest_provider.cc
,
Jun 22 2017
Only trimming HTTP & HTTPS sounds good to me.
,
Jun 22 2017
^ thanks for the confirmation! Some more trimmings are coming soon -- will keep you posted.
,
Jun 23 2017
Hey, I'm working on the trimming of trivial subdomains (m. / www.) This leads to some counter-intuitive results if the match portion is within the part that we trimmed out though. So it seems like we will need to implement a match_in_subdomain flag similar to how we already have a match_in_scheme flag to prevent trimming of schemes that we match in. Thoughts?
,
Jun 23 2017
|match_in_subdomain| will take a bit of work to make it work correctly in HistoryQuick provider (unless Peter lets you do a quick hack). Right now it simply does some string searches for "." and "/" to parse URLs. When I tried to fix bug 595524 and bug 448659 following that same approach, Peter pushed back, saying that the code should be refactored to use the parsed URL structure. I wasn't ready to bite off that refactoring. You're certainly welcome to if you think it's worth it for the subdomain experiment; if you do, you can fix those too bugs at the same time. :-)
,
Jun 23 2017
mpearson - ah oh dear. It seems like for all the URL elision experiments, we want to skip the elision of the match portion is in the elided part -- so doing the full parsing / refactoring seems inevitable. +cc pkasting for his input
,
Jun 28 2017
Hey Emily / Max... We're eliding everything after the host right? So for a URL like: http://foo.com/path/to/something?key=value#ref We remove the query and ref part of the URL too right? So it becomes: http://foo.com/... Based on the mocks, and common sense, that's what we want to do, but just confirming...
,
Jun 28 2017
Well, it would just become "foo.com/..." with the scheme ellided. But yep, sg.
,
Jun 28 2017
Emily - great! Yes, I can confirm the scheme will be elided also.
,
Jun 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94d50d2defeafa336cd6574f43dccbbd69306480 commit 94d50d2defeafa336cd6574f43dccbbd69306480 Author: tommycli <tommycli@chromium.org> Date: Wed Jun 28 22:47:19 2017 Omnibox UI Experiments: Implement elide-after-host experiment. This implements the logic to do the elide-after-host experiment. This isn't hooked up to the feature flag yet (which will be in a followup). This CL elides everything after the host, including the path, query, and ref. BUG= 732582 Review-Url: https://codereview.chromium.org/2961093002 Cr-Commit-Position: refs/heads/master@{#483179} [modify] https://crrev.com/94d50d2defeafa336cd6574f43dccbbd69306480/components/url_formatter/url_formatter.cc [modify] https://crrev.com/94d50d2defeafa336cd6574f43dccbbd69306480/components/url_formatter/url_formatter.h [modify] https://crrev.com/94d50d2defeafa336cd6574f43dccbbd69306480/components/url_formatter/url_formatter_unittest.cc
,
Jun 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72014f6a8ab12897583185fc6c5b4752fcbaa278 commit 72014f6a8ab12897583185fc6c5b4752fcbaa278 Author: tommycli <tommycli@chromium.org> Date: Thu Jun 29 21:42:16 2017 Omnibox UI Experiments: Refactor HTTPS trimming into UrlFormatter. Given that we must implement elide-after-host logic within UrlFormatter, consolidate the other experimental URL trimming logic into the same method. BUG= 732582 Review-Url: https://codereview.chromium.org/2963883002 Cr-Commit-Position: refs/heads/master@{#483497} [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/chrome/browser/ui/cocoa/status_bubble_mac_unittest.mm [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/autocomplete_match.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/autocomplete_match.h [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/autocomplete_match_unittest.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/clipboard_url_provider.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/history_quick_provider.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/history_url_provider.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/titled_url_match_utils.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/omnibox/browser/zero_suggest_provider.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/url_formatter/url_formatter.cc [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/url_formatter/url_formatter.h [modify] https://crrev.com/72014f6a8ab12897583185fc6c5b4752fcbaa278/components/url_formatter/url_formatter_unittest.cc
,
Jul 5 2017
pkasting: I'm reaching the point where I need to refactor ScoredHistoryMatch::GetTopicalityScore to support match_in_subdomain and match_in_path, etc. In the past, you mentioned the way to go about this was to refactor the above to use the standard GURL parsing machinery instead of doing string searching. I investigated this, and it's a bit tricky, since the matches are actually performed on the base::string16 that comes out of bookmarks::CleanUpUrlForMatching that calls url_formatter::FormatUrl. After the URL has been formatted, it's no longer possible to use the standard GURL parsing system AFAIK. An alternative is to modify FormatUrl to also return a vector of offsets for the various parts of the URL as they are formatted and appended to the formatted string. Thoughts?
,
Jul 5 2017
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10b4b19297da4a3cd28944da3134df517946b50b commit 10b4b19297da4a3cd28944da3134df517946b50b Author: tommycli <tommycli@chromium.org> Date: Thu Jul 06 00:24:41 2017 Omnibox UI Experiments: Hook up elide-after-host feature flag. This hooks up the (renamed) elide-after-host feature flag to the actual implementation. BUG= 732582 Review-Url: https://codereview.chromium.org/2961393002 Cr-Commit-Position: refs/heads/master@{#484410} [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/chrome/browser/about_flags.cc [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/chrome/browser/flag_descriptions.h [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/components/omnibox/browser/autocomplete_match.cc [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/components/omnibox/browser/autocomplete_match_unittest.cc [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/components/omnibox/browser/omnibox_field_trial.cc [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/components/omnibox/browser/omnibox_field_trial.h [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/tools/metrics/histograms/enums.xml [modify] https://crrev.com/10b4b19297da4a3cd28944da3134df517946b50b/tools/metrics/histograms/histograms.xml
,
Jul 6 2017
tommycli@: did you notice there is a FormatUrlWithOffsets() function already?
,
Jul 6 2017
mpearson: I totally did not make the connection that the offsets adjustment used for other things could also be used to track where the components end up. That's a great suggestion. Thank you!
,
Jul 6 2017
Looking at the changelist in question (comment #3), I think the following providers were erroneously omitted and thus not going to have their schemes correctly trimmed: * shortcuts provider * navigational suggestions from the suggest server * bookmarks * URLs provided by invoking an omnibox extension (keyword provider) * physical web provider In future changelists / for other UI experimental code, you may want to simply think about / audit all providers to make sure they behave correctly.
,
Jul 6 2017
jdonnelly, mpearson: I initially thought that certain providers (like bookmarks, physical web) should not have their URLs modified, as that would be confusing. (User asks - why is my bookmark URL being modified??) However, it seems that it may be more confusing to have inconsistent behavior. I'll send out a patch to make all the providers use the same behavior.
,
Jul 6 2017
To clarify c#21, I'm mostly thinking about the trivial subdomain elision case, since that actually is a domain with a different 'meaning' than the scheme-removal or path-ellipsize case.
,
Jul 6 2017
> To clarify c#21, I'm mostly thinking about the trivial subdomain elision case And to clarify comment #20, I was thinking more about omitting the scheme :-), though other UI experiments also broadly apply to my comment. I think consistency is most important; I don't want to expose the details of the mess of different suggestion sources to users.
,
Jul 6 2017
mpearson: Okay great. I also talked to Justin and I think we're all on the same page now. I'll be sending a CL out to make them all behave consistently for now. In the future we may want to refine things a bit. for example: for suggestions that we don't have a meaningful title or a description for (maybe clipboard or physical web), we probably won't want to ellipsize the path, since it may make multiple suggestions look identical.
,
Jul 6 2017
You may be right on that (don't elide things that may be needed if the URL doesn't have a meaningful title/description). For the record though, clipboard suggestions have a meaningful title ("Link you copied"). Physical web might too; I'm not sure off the top of my head.
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/92807e9faaff9f742a6f45c946f61931882408c7 commit 92807e9faaff9f742a6f45c946f61931882408c7 Author: tommycli <tommycli@chromium.org> Date: Thu Jul 06 18:55:12 2017 Omnibox UI Experiments: Strip trivial subdomains This revised patch strips the trivial subdomains while preserving the registry and domain. It also correctly handles the wwww.foo.com (4 w's), which the previous version did not. Previous version: https://codereview.chromium.org/2939423003/ BUG= 732582 Review-Url: https://codereview.chromium.org/2966233002 Cr-Commit-Position: refs/heads/master@{#484695} [modify] https://crrev.com/92807e9faaff9f742a6f45c946f61931882408c7/components/omnibox/browser/autocomplete_match.cc [modify] https://crrev.com/92807e9faaff9f742a6f45c946f61931882408c7/components/omnibox/browser/autocomplete_match_unittest.cc [modify] https://crrev.com/92807e9faaff9f742a6f45c946f61931882408c7/components/url_formatter/url_formatter.cc [modify] https://crrev.com/92807e9faaff9f742a6f45c946f61931882408c7/components/url_formatter/url_formatter.h [modify] https://crrev.com/92807e9faaff9f742a6f45c946f61931882408c7/components/url_formatter/url_formatter_unittest.cc
,
Jul 7 2017
I discussed the trivial subdomain elision with jdonnelly this morning, and we had these thoughts: - Justin tested the www and m cases for the Alexa top 25. - Stripping the leading www. was great! All positives, no problems. - Stripping off the leading m. might be kind of problematic. For instance, m.intl.taobao.com is the mobile site, but intl.taobao.com doesn't resolve. - Stripping off subdomains in the middle of hosts (i.e. en.m.wikipedia.org) works in that case, but also seems ... aggressive.. since it could yield a host that doesn't exist. - Basically we were worried about suggesting a URL that doesn't actually resolve / has a different semantic meaning. Some questions we had: 1. Strip the leading www. from suggest URLs, as that's uncontroversial. 2. If the user visits m.facebook.com a lot from their phone, and those history entries are synced, does m.facebook.com appear as a completion on their desktop? 3. Instead of stripping parts of the subdomain, can we instead use the redirect chain in a smart way? i.e., users on both desktop and mobile always type "facebook.com" even though it's a redirect to "m.facebook.com" on mobile. Can we always make the suggestion URL the original typed URL instead of the final one in the chain?
,
Jul 7 2017
> - Basically we were worried about suggesting a URL that doesn't actually > resolve / has a different semantic meaning. I thought we were only stripping them in the dropdown display? How could that cause any problems? If you select them, they'll go to the right place (the full |destination_url|). And when the user arrows down to the entry, they'll get the |fill_into_edit| field to edit more, not the displayed text, and this will be unstripped too. So I don't see how displaying URLs in the dropdown in whatever form you want will cause any issues. What am I missing? > 2. If the user visits m.facebook.com a lot from their phone, and those > history entries are synced, does m.facebook.com appear as a completion on their > desktop? I would think so, but I cannot reproduce this, so maybe not?
,
Jul 7 2017
> I thought we were only stripping them in the dropdown display? That's correct. > How could that cause any problems? It won't cause any functional problems, as you describe. Our concern is over presenting misleading information to the user. For example, once someone visits taobao.com on their phone a couple times, now we present intl.taobao.com in the suggestion. It's not unlikely that the user might get used to seeing that and at some point type it in expecting that to actually go somewhere.
,
Jul 7 2017
> Justin tested the www and m cases for the Alexa top 25 At some point we should do a much broader survey.
,
Jul 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37e685b305e4436d76e1986bcba5b5ffa092cc41 commit 37e685b305e4436d76e1986bcba5b5ffa092cc41 Author: Tommy C. Li <tommycli@chromium.org> Date: Mon Jul 10 19:01:23 2017 Omnibox UI Experiments: Tweak provider usage - Enables on Physical Web - Enables for search suggestions - Disables on all fill_into_edit fields (which shouldn't have these) Bug: 732582 Change-Id: I4828bbf37967f87578a5d48d393b7c0e312a5ecb Reviewed-on: https://chromium-review.googlesource.com/563601 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#485333} [modify] https://crrev.com/37e685b305e4436d76e1986bcba5b5ffa092cc41/components/omnibox/browser/autocomplete_match.h [modify] https://crrev.com/37e685b305e4436d76e1986bcba5b5ffa092cc41/components/omnibox/browser/history_url_provider.cc [modify] https://crrev.com/37e685b305e4436d76e1986bcba5b5ffa092cc41/components/omnibox/browser/physical_web_provider.cc [modify] https://crrev.com/37e685b305e4436d76e1986bcba5b5ffa092cc41/components/omnibox/browser/search_suggestion_parser.cc
,
Jul 10 2017
So, from the list in comment #20, the latest changelist fixes some. > * navigational suggestions from the suggest server Fixed. > * physical web provider Fixed. > * bookmarks Untouched, but should already work. > * URLs provided by invoking an omnibox extension (keyword provider) Untouched. This is probably intentional--we shouldn't be messing with these. > * shortcuts provider Untouched. It probably should be touched, else you get inconsistency in the UI, where shortcut provider suggestions display the same match_contents as the previous time they were displayed (thus exempting themselves from the UI changes).
,
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
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02c85c5529c3d279db4ef93bb072f8ef440dcd4b commit 02c85c5529c3d279db4ef93bb072f8ef440dcd4b Author: Tommy C. Li <tommycli@chromium.org> Date: Wed Jul 12 21:57:20 2017 Omnibox UI Experiments: Fix path elision behavior on HQP. HQP currently shares a FormatUrl call between the match.fill_into_edit and match.contents calls. This is no longer appropriate given that the path may be elided. Per discussions with pkasting, none of the destructive elisions should be applied to the match.fill_into_edit fields. Bug: 732582 Change-Id: I91b1181edaa2d3b9f150a9313951bf6ff59103a4 Reviewed-on: https://chromium-review.googlesource.com/567236 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#486117} [modify] https://crrev.com/02c85c5529c3d279db4ef93bb072f8ef440dcd4b/components/omnibox/browser/history_quick_provider.cc
,
Jul 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a4241c698a71a5cc10ec6135060f4d99af2b96c commit 7a4241c698a71a5cc10ec6135060f4d99af2b96c Author: tommycli <tommycli@chromium.org> Date: Thu Jul 13 01:37:32 2017 base::OffsetAdjuster: Refactor offset limiting logic into the base::OffsetAdjuster Previously, almost all usages of OffsetAdjuster applied a post-processing step to clamp the resulting offsets to a "sane" range. This refactor moves the clamping into the base function, but as an optional parameter. This also reduces the API surface by removing the LimitOffset class. BUG= 732582 Review-Url: https://codereview.chromium.org/2953943003 Cr-Commit-Position: refs/heads/master@{#486205} [modify] https://crrev.com/7a4241c698a71a5cc10ec6135060f4d99af2b96c/base/strings/utf_offset_string_conversions.cc [modify] https://crrev.com/7a4241c698a71a5cc10ec6135060f4d99af2b96c/base/strings/utf_offset_string_conversions.h [modify] https://crrev.com/7a4241c698a71a5cc10ec6135060f4d99af2b96c/base/strings/utf_offset_string_conversions_unittest.cc [modify] https://crrev.com/7a4241c698a71a5cc10ec6135060f4d99af2b96c/components/url_formatter/url_formatter.cc
,
Jul 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/75d530de0768b9e95855f936ec5779091603d579 commit 75d530de0768b9e95855f936ec5779091603d579 Author: Tommy C. Li <tommycli@chromium.org> Date: Mon Jul 17 16:56:19 2017 Omnibox UI Experiments: Add match_in_subdomain and match_after_host. Adding match_in_subdomain and match_after_host to support Omnibox UI experiments. We have UI experiments that elide parts of the subdomain and the path, query, and ref. These shouldn't be elided if they are part of the match to the user-typed terms. This CL has no immediate effect, but adds the match_in_subdomain and match_after_host fields to be later integrated into the Omnibox UI experiments. Bug: 732582 Change-Id: I870d0192eabeae17c79f842bb51928646a3c504e Reviewed-on: https://chromium-review.googlesource.com/568636 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#487121} [modify] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/history/core/browser/BUILD.gn [modify] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/BUILD.gn [rename] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/history_match.cc [rename] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/history_match.h [modify] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/history_url_provider.cc [modify] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/history_url_provider.h [modify] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/scored_history_match.cc [modify] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/scored_history_match.h [modify] https://crrev.com/75d530de0768b9e95855f936ec5779091603d579/components/omnibox/browser/scored_history_match_unittest.cc
,
Jul 17 2017
I commented on the most recently changelist about some concerns I have about the implementation. Commenting here too in case people ignore comments on submitted changelists.
,
Jul 26 2017
Can you share access to document in task?
,
Aug 1 2017
a-v-y@yandex-team.ru: Apologies, but that document has internal planning details that we can't share publicly. Note that the general idea here is that these are UI experiments. We do not currently have plans to ship any of these features. They are being implemented as experimental options so that we can test them out and see how they function in our own day-to-day browsing. You can try these out yourself if you have Canary installed by enabling the following flags: chrome://flags/#omnibox-ui-hide-suggestion-url-scheme chrome://flags/#omnibox-ui-hide-suggestion-url-trivial-subdomains chrome://flags/#omnibox-ui-elide-suggestion-url-after-host
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8737d4db2a72fec5ca07e6f5d3a5fc2418185c6 commit b8737d4db2a72fec5ca07e6f5d3a5fc2418185c6 Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Aug 01 21:02:02 2017 Omnibox: Improve ScoredHistoryMatch handling of match_after_host etc. Previously ScoredHistoryMatch did not correctly flag match_after_host and match_in_subdomain when the term spans multiple components. This CL fixes that. Bug: 732582 Change-Id: I240b9681a9d5c5ddea95de9aaa6fee4ed35d258e Reviewed-on: https://chromium-review.googlesource.com/595040 Reviewed-by: Mark Pearson <mpearson@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#491107} [modify] https://crrev.com/b8737d4db2a72fec5ca07e6f5d3a5fc2418185c6/components/omnibox/browser/scored_history_match.cc [modify] https://crrev.com/b8737d4db2a72fec5ca07e6f5d3a5fc2418185c6/components/omnibox/browser/scored_history_match_unittest.cc
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a46e6389af727d21421651e93bc9a2200a28876f commit a46e6389af727d21421651e93bc9a2200a28876f Author: Tommy C. Li <tommycli@chromium.org> Date: Tue Aug 01 23:26:27 2017 Omnibox: Hook up match_in_subdomain, etc. to URL formatting. This CL prevents the match part of the URL from being elided by the experimental URL formatting flags. Bug: 732582 Change-Id: Ic08a8cad845f4d4581d8f6310e59423191d4bd28 Reviewed-on: https://chromium-review.googlesource.com/594722 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#491150} [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/autocomplete_match.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/autocomplete_match.h [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/autocomplete_match_unittest.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/clipboard_url_provider.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/history_quick_provider.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/history_url_provider.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/physical_web_provider.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/search_suggestion_parser.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/titled_url_match_utils.cc [modify] https://crrev.com/a46e6389af727d21421651e93bc9a2200a28876f/components/omnibox/browser/zero_suggest_provider.cc
,
Aug 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3ae4e82c71fa98f2b03ba80f4d610eea1adc76c2 commit 3ae4e82c71fa98f2b03ba80f4d610eea1adc76c2 Author: Tommy C. Li <tommycli@chromium.org> Date: Thu Aug 03 18:47:43 2017 Omnibox: Fix HistoryUrlProvider's match_in_foo implementation. Fixes HistoryUrlProvider's match_in_foo implementation, and adds tests. Bug: 732582 Change-Id: I5f63a74bb02a6cc97c771b26080217ea9559eb3b Reviewed-on: https://chromium-review.googlesource.com/597170 Commit-Queue: Tommy Li <tommycli@chromium.org> Reviewed-by: Mark Pearson <mpearson@chromium.org> Cr-Commit-Position: refs/heads/master@{#491804} [modify] https://crrev.com/3ae4e82c71fa98f2b03ba80f4d610eea1adc76c2/components/omnibox/browser/history_url_provider.cc [modify] https://crrev.com/3ae4e82c71fa98f2b03ba80f4d610eea1adc76c2/components/omnibox/browser/history_url_provider_unittest.cc
,
Aug 9 2017
This is ready for PM / UX testing. I'm sure there are still rough edges, but that can be deferred when we have a clearer idea of launch-no-launch. Those rough edges should be separate bugs.
,
Aug 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/affe1c851d76ea61758ee943091f0a06392b5d72 commit affe1c851d76ea61758ee943091f0a06392b5d72 Author: Tommy C. Li <tommycli@chromium.org> Date: Fri Aug 18 23:28:23 2017 Omnibox UI Experiments: Add URL trimming flags to Mobile Makes the URL trimming flags available on Android and iOS as well. Bug: 732582 Change-Id: I2fdb5ab3869049bca2d409190eccb74d3a3e9fda Reviewed-on: https://chromium-review.googlesource.com/617100 Reviewed-by: Justin Donnelly <jdonnelly@chromium.org> Commit-Queue: Tommy Li <tommycli@chromium.org> Cr-Commit-Position: refs/heads/master@{#495743} [modify] https://crrev.com/affe1c851d76ea61758ee943091f0a06392b5d72/chrome/browser/about_flags.cc
,
Aug 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33d712ed02860e421f577dbad7ec65423a651615 commit 33d712ed02860e421f577dbad7ec65423a651615 Author: Justin Donnelly <jdonnelly@google.com> Date: Wed Aug 23 21:32:51 2017 Add URL elision flags to the iOS version of chrome://flags. Bug: 732582 Change-Id: I665fd1874541146da691dec462fd3409bfdb54b9 Reviewed-on: https://chromium-review.googlesource.com/629198 Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: Justin Donnelly <jdonnelly@chromium.org> Cr-Commit-Position: refs/heads/master@{#496808} [modify] https://crrev.com/33d712ed02860e421f577dbad7ec65423a651615/ios/chrome/browser/BUILD.gn [modify] https://crrev.com/33d712ed02860e421f577dbad7ec65423a651615/ios/chrome/browser/about_flags.mm [modify] https://crrev.com/33d712ed02860e421f577dbad7ec65423a651615/ios/chrome/browser/ios_chrome_flag_descriptions.cc [modify] https://crrev.com/33d712ed02860e421f577dbad7ec65423a651615/ios/chrome/browser/ios_chrome_flag_descriptions.h |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by tommycli@chromium.org
, Jun 12 2017