Closed
Bug 1481319
Opened 6 years ago
Closed 6 years ago
"http://x" incorrectly loads search bookmark with keyword "x"
Categories
(Firefox :: Address Bar, defect, P2)
Firefox
Address Bar
Tracking
()
RESOLVED
FIXED
Firefox 63
People
(Reporter: mbrubeck, Assigned: mbrubeck)
References
Details
(Keywords: regression, Whiteboard: [fxsearch])
Attachments
(1 file)
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).
Assignee | ||
Comment 1•6 years ago
|
||
This is happening at this line: https://searchfox.org/mozilla-central/rev/51268dcbdff0f6f4a5cff7986df0f616efc5bcfd/toolkit/components/places/UnifiedComplete.js#1539 `this._searchTokens[0]` 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".
Updated•6 years ago
|
Component: Places → Address Bar
Keywords: regression,
regressionwindow-wanted
Priority: -- → P2
Product: Toolkit → Firefox
Whiteboard: [fxsearch]
Assignee | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•6 years ago
|
||
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
![]() |
||
Comment 4•6 years ago
|
||
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
Comment 6•6 years ago
|
||
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+
Assignee | ||
Comment 7•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3176857bbc04a937cebc63c2b9ec0d5514113d3
Pushed by mbrubeck@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7645fd7dc1a4 Include prefix when matching bookmark keywords. r=mak
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7645fd7dc1a4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Comment 10•6 years ago
|
||
Given how old the regression is, doesn't seem worth backporting to ESR60. Did you want to nominate for Beta approval?
Assignee | ||
Comment 11•6 years ago
|
||
I don't think this is critical to backport to beta, but I'll defer to mak if thinks we should.
Flags: needinfo?(mbrubeck)
Comment 12•6 years ago
|
||
yep, not critical, it's an edge case.
You need to log in
before you can comment on or make changes to this bug.
Description
•