Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(93)

Issue 1902263002: Support fixed prefixes and suffixes when extracting terms from search template URLs. (Closed)

Created:
4 years, 8 months ago by jbroman
Modified:
4 years, 8 months ago
Reviewers:
Peter Kasting
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Support fixed prefixes and suffixes when extracting terms from search template URLs. This causes autocompletions for custom search engines where the query parameter has some prefix or suffix to work correctly, instead of duplicating it. BUG=602450 TEST=components_unittests:TemplateURLTest.ExtractSearchTermsWithPrefixAndSuffix Committed: https://crrev.com/508320252e351bc010cc25d837b55baf6d0d22c4 Cr-Commit-Position: refs/heads/master@{#388958}

Patch Set 1 #

Total comments: 13

Patch Set 2 : review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+83 lines, -17 lines) Patch
M components/search_engines/template_url.h View 1 chunk +2 lines, -0 lines 0 comments Download
M components/search_engines/template_url.cc View 1 5 chunks +55 lines, -17 lines 0 comments Download
M components/search_engines/template_url_unittest.cc View 1 1 chunk +26 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
jbroman
One could imagine being yet more general, but this at least solves my case, and ...
4 years, 8 months ago (2016-04-19 23:21:13 UTC) #2
Peter Kasting
https://codereview.chromium.org/1902263002/diff/1/components/search_engines/template_url.cc File components/search_engines/template_url.cc (right): https://codereview.chromium.org/1902263002/diff/1/components/search_engines/template_url.cc#newcode102 components/search_engines/template_url.cc:102: base::StringPiece needle, Nit: I'd avoid references to English idioms ...
4 years, 8 months ago (2016-04-21 19:48:58 UTC) #3
jbroman
https://codereview.chromium.org/1902263002/diff/1/components/search_engines/template_url.cc File components/search_engines/template_url.cc (right): https://codereview.chromium.org/1902263002/diff/1/components/search_engines/template_url.cc#newcode102 components/search_engines/template_url.cc:102: base::StringPiece needle, On 2016/04/21 at 19:48:57, Peter Kasting wrote: ...
4 years, 8 months ago (2016-04-21 21:01:42 UTC) #4
Peter Kasting
LGTM https://codereview.chromium.org/1902263002/diff/1/components/search_engines/template_url.cc File components/search_engines/template_url.cc (right): https://codereview.chromium.org/1902263002/diff/1/components/search_engines/template_url.cc#newcode102 components/search_engines/template_url.cc:102: base::StringPiece needle, On 2016/04/21 21:01:41, jbroman wrote: > ...
4 years, 8 months ago (2016-04-21 22:29:46 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1902263002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1902263002/20001
4 years, 8 months ago (2016-04-21 22:59:01 UTC) #7
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 8 months ago (2016-04-21 23:53:13 UTC) #8
commit-bot: I haz the power
4 years, 8 months ago (2016-04-22 19:41:49 UTC) #10
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/508320252e351bc010cc25d837b55baf6d0d22c4
Cr-Commit-Position: refs/heads/master@{#388958}

Powered by Google App Engine
This is Rietveld 408576698