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".
Component: Places → Address Bar
Keywords: regression, regressionwindow-wanted
Priority: -- → P2
Product: Toolkit → Firefox
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
status-firefox61: --- → wontfix
status-firefox62: --- → fix-optional
status-firefox-esr52: --- → wontfix
status-firefox-esr60: --- → fix-optional
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
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox63: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Given how old the regression is, doesn't seem worth backporting to ESR60. Did you want to nominate for Beta approval?
status-firefox-esr60: fix-optional → wontfix
I don't think this is critical to backport to beta, but I'll defer to mak if thinks we should.
yep, not critical, it's an edge case.
status-firefox62: fix-optional → wontfix
You need to log in before you can comment on or make changes to this bug.