Exactly-Typed Non-Substituting Keywords Aren't Made the Default Match |
||||||||
Issue descriptionSomething seems to have regressed between r518583 (green test run here: https://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x64_rel_ng/builds/32664) and r520082 (red test run here: https://build.chromium.org/p/tryserver.chromium.win/builders/win10_chromium_x64_rel_ng/builds/36810). The test is now failing consistently with: [ RUN ] OmniboxViewTest.NonSubstitutingKeywordTest ../../chrome/browser/ui/omnibox/omnibox_view_browsertest.cc(1310): error: Expected: AutocompleteMatchType::HISTORY_KEYWORD Which is: 4 To be equal to: popup_model->result().default_match()->type Which is: 6 [5184:2328:1129/061833.120:INFO:chrome_cryptauth_service.cc(222)] Profile is not authenticated yet; waiting before starting CryptAuth managers. [5184:2328:1129/061833.145:WARNING:url_request_context_getter.cc(43)] URLRequestContextGetter leaking due to no owning thread. [ FAILED ] OmniboxViewTest.NonSubstitutingKeywordTest, where TypeParam = and GetParam() = (799 ms) See the green and red tryjobs on https://chromium-review.googlesource.com/c/chromium/src/+/784937/2. Assigning to some random amateur in chrome/browser/ui/omnibox/OWNERS for triage and assignment.
,
Dec 1 2017
I haven't even tested to see if this is the cause of the error, but I think I know what's going on. On this line https://cs.chromium.org/chromium/src/components/omnibox/browser/keyword_provider.cc?l=348 instead of input.allow_exact_keyword_match() we should use input.allow_exact_keyword_match() || !template_url->SupportsReplacement(model_->search_terms_data())
,
Dec 1 2017
Thank you for picking this up so quickly!
,
Dec 1 2017
> Thank you for picking this up so quickly! You're welcome. Thanks for reporting. This is a real (user-facing) bug. I'm glad you tried to revive the tests and thus discovered it. Repro steps: 1. Start chrome. 2. Create a new "search" engine that doesn't actually do a search. For example, create an engine "Testing Engine" with keyword "test" that goes to http://example.com/. Note the lack of the "%s" in the "search" URL. 3. Type "test" in the omnibox Expected results: 4. "Testing engine" is the default match. If you press enter, you go to example.com. Actual results: 4. The default match is "test". If you press enter, you go to a google search for "test". The second match was for "Testing engine".
,
Dec 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0d2d779e0c03c8c7a3df30b33a33f494cf148629 commit 0d2d779e0c03c8c7a3df30b33a33f494cf148629 Author: Mark Pearson <mpearson@chromium.org> Date: Sat Dec 02 06:34:44 2017 Omnibox - Exactly-Typed Non-Substituting Keywords Should Be Default This was broken by https://chromium.googlesource.com/chromium/src/+/31c2066fb04ade8b1474c27054f425fe8a72d04f Tested interactively using repro steps on bug. Also re-enables programmatic test for this bug. Bug: 789913 Change-Id: I43a59ef112da6f1e69c30a46c55586d5f9280f41 Reviewed-on: https://chromium-review.googlesource.com/804676 Commit-Queue: Mark Pearson <mpearson@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/master@{#521203} [modify] https://crrev.com/0d2d779e0c03c8c7a3df30b33a33f494cf148629/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc [modify] https://crrev.com/0d2d779e0c03c8c7a3df30b33a33f494cf148629/components/omnibox/browser/keyword_provider.cc
,
Dec 2 2017
,
Dec 2 2017
Issue 791272 has been merged into this issue.
,
Dec 3 2017
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/743227c56e6d75d2689b7e8220654356df1e7843 commit 743227c56e6d75d2689b7e8220654356df1e7843 Author: Mark Pearson <mpearson@chromium.org> Date: Mon Dec 04 17:52:23 2017 Omnibox - Exactly-Typed Non-Substituting Keywords Should Be Default This was broken by https://chromium.googlesource.com/chromium/src/+/31c2066fb04ade8b1474c27054f425fe8a72d04f Tested interactively using repro steps on bug. Also re-enables programmatic test for this bug. Bug: 789913 Change-Id: I43a59ef112da6f1e69c30a46c55586d5f9280f41 Reviewed-on: https://chromium-review.googlesource.com/804676 Commit-Queue: Mark Pearson <mpearson@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#521203}(cherry picked from commit 0d2d779e0c03c8c7a3df30b33a33f494cf148629) Reviewed-on: https://chromium-review.googlesource.com/806795 Reviewed-by: Mark Pearson <mpearson@chromium.org> Cr-Commit-Position: refs/branch-heads/3282@{#14} Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} [modify] https://crrev.com/743227c56e6d75d2689b7e8220654356df1e7843/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc [modify] https://crrev.com/743227c56e6d75d2689b7e8220654356df1e7843/components/omnibox/browser/keyword_provider.cc
,
Dec 4 2017
,
Dec 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e1ccd58125c75660a7790724ea5faed78a055d0c commit e1ccd58125c75660a7790724ea5faed78a055d0c Author: Greg Thompson <grt@chromium.org> Date: Mon Dec 04 19:49:53 2017 Enable interactive_ui_tests on Windows that now run mostly reliably. r516772 was the last of many commits that resolved a number of problems that made this test suite flaky on Windows. This CL enables tests that had been disabled over time due to flakes, yet now seem to run okay. BUG= 133341 ,751031, 751543 , 764415 , 789913 This CL was uploaded by git cl split. R=sky@chromium.org Change-Id: I753c741a9194f489f072cd86d72407a1294150c3 Reviewed-on: https://chromium-review.googlesource.com/784937 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Greg Thompson <grt@chromium.org> Cr-Commit-Position: refs/heads/master@{#521416} [modify] https://crrev.com/e1ccd58125c75660a7790724ea5faed78a055d0c/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16ac914c0806c5885ef112f621b5ae62d13fdbf3 commit 16ac914c0806c5885ef112f621b5ae62d13fdbf3 Author: Greg Thompson <grt@chromium.org> Date: Tue Dec 05 10:24:50 2017 Revert "Enable interactive_ui_tests on Windows that now run mostly reliably." This reverts commit e1ccd58125c75660a7790724ea5faed78a055d0c. Reason for revert: Widespread "Check failed: false. Observers can only be added once!" failures. Original change's description: > Enable interactive_ui_tests on Windows that now run mostly reliably. > > r516772 was the last of many commits that resolved a number of problems > that made this test suite flaky on Windows. This CL enables tests that > had been disabled over time due to flakes, yet now seem to run okay. > > BUG= 133341 ,751031, 751543 , 764415 , 789913 > This CL was uploaded by git cl split. > > R=sky@chromium.org > > Change-Id: I753c741a9194f489f072cd86d72407a1294150c3 > Reviewed-on: https://chromium-review.googlesource.com/784937 > Reviewed-by: Scott Violet <sky@chromium.org> > Commit-Queue: Greg Thompson <grt@chromium.org> > Cr-Commit-Position: refs/heads/master@{#521416} TBR=sky@chromium.org,grt@chromium.org Change-Id: I33b192c628fdfebe9cfd93b71abd3b394faad9e5 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 133341 , 751031, 751543 , 764415 , 789913 Reviewed-on: https://chromium-review.googlesource.com/808204 Reviewed-by: Greg Thompson <grt@chromium.org> Commit-Queue: Greg Thompson <grt@chromium.org> Cr-Commit-Position: refs/heads/master@{#521652} [modify] https://crrev.com/16ac914c0806c5885ef112f621b5ae62d13fdbf3/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc
,
Jan 24 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by mpear...@chromium.org
, Nov 30 2017Owner: mpear...@chromium.org