46 bytes, text/x-phabricator-request
|Details | Review|
Steps to reproduce: 1. Create a bookmark with URL "http://google.com/?q=%s" and keyword "foobarbaz". 2. Type "http://foobarbaz" into the location bar and press Enter. Expected result: * Firefox navigates to the URL "http://foobarbaz". Actual result: * Firefox navigates to the URL "http://google.com/?q=barbaz" (note that "foo" is cut off).
This is happening at this line: https://searchfox.org/mozilla-central/rev/51268dcbdff0f6f4a5cff7986df0f616efc5bcfd/toolkit/components/places/UnifiedComplete.js#1539 `this._searchTokens` contains the URL after its prefix was stripped ("foobarbaz"), while `this._trimmedOriginalSearchString` contains the whole URL including the prefix ("http://foobarbaz"). The line above attempts to strip off the keyword, but doesn't account for the length of the prefix, so it strips the first 10 characters ("http://foo") leaving "barbaz".
This patch restores the stripped prefix before matching against bookmark keywords. This means that entering "foo:bar" will no longer activate a bookmark with keyword "bar", but it will now match a bookmark with keyword "foo:bar". Alternately, this function could simply return false if the original search string contains any keyword. This would make it impossible to use any keyword that contains ":".
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
It was gradually broken. Before landing of Bug 1306639, Firefox navigates to "http://foobarbaz" as expected. Between landing of Bug 1306639 and Bug 1310737, Firefox navigates to "https://www.google.com/?q=". After landing of Bug 1310737, Firefox navigates to "http://google.com/?q=barbaz" #1 regression window(Firefox navigates to "https://www.google.com/?q="): https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=64cd7d87e78cf2e4b919aad10fb9f4962834b004&tochange=70d19ed29c980ba3d6825e5286bda08fa7dc676f Regressed by: 70d19ed29c98 Marco Bonardo — Bug 1306639 - Searching in locationbar by typing something and pressing enter is not accounted in telemetry. r=adw #2 regression window(Firefox navigates to "http://google.com/?q=barbaz"): https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fc6a63bedaec6e34bb3b2491b497b4f131387dab&tochange=74b268465bc724c303f44de6fd3bb939c56f52de Regressed by: 74b268465bc7 Marco Bonardo — Bug 1310737 - Urlbar doesn't properly handle %S and POST keywords. r=adw
Thank you for checking the range.
Comment on attachment 8998037 [details] Bug 1481319 - Include prefix when matching bookmark keywords. r?mak Marco Bonardo [::mak] (Away 9-26 Aug) has approved the revision. https://phabricator.services.mozilla.com/D2804
Attachment #8998037 - Flags: review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/7645fd7dc1a4 Include prefix when matching bookmark keywords. r=mak
Given how old the regression is, doesn't seem worth backporting to ESR60. Did you want to nominate for Beta approval?
I don't think this is critical to backport to beta, but I'll defer to mak if thinks we should.
You need to log in before you can comment on or make changes to this bug.