New issue
Advanced search Search tips

Issue 834876 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Figure out why QiO doesn't always work for all search engines

Project Member Reported by mariakho...@chromium.org, Apr 19 2018

Issue description

From treib@'s response to PRD, sounds like sometimes Query in Omnibox doesn't trigger for a search engine set from settings.

Figure out how that happens. Maybe it has to do with sync from desktop? Some sort of locality issue?
 
It looks like the prepopulated engine templates are a little too rigid. Searches from the omnibox with de.search.yahoo.com as my DSE work just fine, and subsequent refinements from the omnibox work just fine as well.

The problem is when refining the search from the input field on the page, I get a URL that doesn't match the templates set out in prepopulated_engines.json.
It looks like in this case, it's because it appends a bunch of stuff to the "/search" like "/search;_ylt=athousandcharacters?" so if we could wildcard chunks of the templates it would be alright, since it functions if you make it "/search?" again.
Project Member

Comment 3 by bugdroid1@chromium.org, May 8 2018

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

commit c88820a8aed383ccb4217443b9a47dd56ae92d01
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Tue May 08 16:16:02 2018

Relax URL matching in TemplateUrl by adding wildcards.

Adds the ability to use {google:wildCard} in the path portion of the
template URLs in prepopulated_engines.json. If used, the path is
separated into a prefix/suffix for comparison instead of using a direct
string comparison.

Bug:  834876 
Change-Id: I7425ffbdd88d6ad0a958d9b32a4eb30ffc235dfe
Reviewed-on: https://chromium-review.googlesource.com/1029200
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556820}
[modify] https://crrev.com/c88820a8aed383ccb4217443b9a47dd56ae92d01/components/search_engines/template_url.cc
[modify] https://crrev.com/c88820a8aed383ccb4217443b9a47dd56ae92d01/components/search_engines/template_url.h
[modify] https://crrev.com/c88820a8aed383ccb4217443b9a47dd56ae92d01/components/search_engines/template_url_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 8 2018

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

commit c4c35ff00a2d383ef1f2a4f432c85c0e56438961
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue May 08 19:54:28 2018

Revert "Relax URL matching in TemplateUrl by adding wildcards."

This reverts commit c88820a8aed383ccb4217443b9a47dd56ae92d01.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 556820 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2M4ODgyMGE4YWVkMzgzY2NiNDIxNzQ0M2I5YTQ3ZGQ1NmFlOTJkMDEM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20MSan%20Tests/9672

Sample Failed Step: components_unittests

Original change's description:
> Relax URL matching in TemplateUrl by adding wildcards.
> 
> Adds the ability to use {google:wildCard} in the path portion of the
> template URLs in prepopulated_engines.json. If used, the path is
> separated into a prefix/suffix for comparison instead of using a direct
> string comparison.
> 
> Bug:  834876 
> Change-Id: I7425ffbdd88d6ad0a958d9b32a4eb30ffc235dfe
> Reviewed-on: https://chromium-review.googlesource.com/1029200
> Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#556820}

Change-Id: Idf00ed1729a913ce1bcde54670ddcb7e64405e27
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  834876 
Reviewed-on: https://chromium-review.googlesource.com/1050583
Cr-Commit-Position: refs/heads/master@{#556935}
[modify] https://crrev.com/c4c35ff00a2d383ef1f2a4f432c85c0e56438961/components/search_engines/template_url.cc
[modify] https://crrev.com/c4c35ff00a2d383ef1f2a4f432c85c0e56438961/components/search_engines/template_url.h
[modify] https://crrev.com/c4c35ff00a2d383ef1f2a4f432c85c0e56438961/components/search_engines/template_url_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 10 2018

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

commit 47ac4d8bbf2bffd53c00cbb7e683b9dd5084f4e4
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu May 10 22:40:00 2018

Reland "Relax URL matching in TemplateUrl by adding wildcards."

This reverts commit c4c35ff00a2d383ef1f2a4f432c85c0e56438961.

Makes sure to initialize path_wildcard_present_ in
TemplateURLRef::InvalidateCacheValues to fix use of uninitialized value
errors.

Bug:  834876 
Change-Id: Iaf1687045ae037a9d2dd2a2accdc1856ef0055ae
Reviewed-on: https://chromium-review.googlesource.com/1050737
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557706}
[modify] https://crrev.com/47ac4d8bbf2bffd53c00cbb7e683b9dd5084f4e4/components/search_engines/template_url.cc
[modify] https://crrev.com/47ac4d8bbf2bffd53c00cbb7e683b9dd5084f4e4/components/search_engines/template_url.h
[modify] https://crrev.com/47ac4d8bbf2bffd53c00cbb7e683b9dd5084f4e4/components/search_engines/template_url_unittest.cc

Labels: QIB-te
Listing the example search engines where Query in Omnibox is not working,
mail.ru
sogou.com
so.com
baidu.com

Thanks Ashwini! Let us know if you find more, we're fixing up the cases where the URL templates are not recognized as search result pages. Should be fixed in M-68.
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 8 2018

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

commit 4a9479ae1757075b86f6aa803dfe4b33028b2b37
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Fri Jun 08 18:24:08 2018

Update prepopulated_engines.json to correctly identify more SRP URLs.

Certain search providers have a search URL that differs on mobile from
what we have in prepopulated_engines.json. Using alternate_urls and
the google:pathWildcard marker this CL adds/fixes several search URLs.

Additionally, all of the Yahoo! are broken across the board, which this
fixes. Query in Omnibox is fixed for all currently known broken cases.

Bug:  834876 
Change-Id: I78955ccbd2079945eeb4542c962fe0673ae4c65c
Reviewed-on: https://chromium-review.googlesource.com/1060413
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565694}
[modify] https://crrev.com/4a9479ae1757075b86f6aa803dfe4b33028b2b37/components/search_engines/prepopulated_engines.json

Status: Fixed (was: Assigned)

Sign in to add a comment