Last Comment Bug 720110 - URL autocomplete breaks keyword bookmarks
: URL autocomplete breaks keyword bookmarks
: regression
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 12 Branch
: All All
-- normal with 1 vote (vote)
: Firefox 12
Assigned To: Marco Bonardo [::mak]
: Marco Bonardo [::mak]
Depends on:
Blocks: 659437 566489 715133
  Show dependency treegraph
Reported: 2012-01-21 03:05 PST by Andrea
Modified: 2012-01-25 07:26 PST (History)
14 users (show)
mak77: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v1.0 (1.35 KB, patch)
2012-01-23 17:57 PST, Marco Bonardo [::mak]
no flags Details | Diff | Splinter Review
patch v1.1 (1.35 KB, patch)
2012-01-23 18:21 PST, Marco Bonardo [::mak]
no flags Details | Diff | Splinter Review
patch v1.1 (11.04 KB, patch)
2012-01-24 10:12 PST, Marco Bonardo [::mak] review+
Details | Diff | Splinter Review

Description User image Andrea 2012-01-21 03:05:57 PST
This is an edge case of the new URL autocomplete feature (see bug 659437, c0).

Steps to reproduce:
1) bookmark a page, e.g., and assign it a keywork, say "fb"
2) visit or any other url with domain name starting with "fb" to add it to your browsing history
3) type "fb" in the urlbar and hit ENTER

Expected results:
fb is recognized as a keyword, Firefox opens the associated bookmark,

Actual results:
Firefox autocompletes the URL with and gets you there as you hit ENTER.

I think that keywords should have priority over URLs, since they are assigned by the user.
Comment 1 User image bogas04 2012-01-22 00:22:01 PST
In my opinion keywords hold higher priority and in anycase you still can press down arrow and choose desired alternative
Comment 2 User image Marco Bonardo [::mak] 2012-01-23 11:10:50 PST
I think it will be easier to fix this from the Places autocomplete implementation side, by not returning a result if the search term matches a keyword.
Comment 3 User image Marco Bonardo [::mak] 2012-01-23 17:57:56 PST
Created attachment 590958 [details] [diff] [review]
patch v1.0

needs a test yet, though, since all the inline ac harness is missing, it should probably just be added in bug 715133. Will check what's the best way on, if work on those tests started or not.
Comment 4 User image Marco Bonardo [::mak] 2012-01-23 18:21:04 PST
Created attachment 590967 [details] [diff] [review]
patch v1.1
Comment 5 User image Marco Bonardo [::mak] 2012-01-24 10:12:50 PST
Created attachment 591152 [details] [diff] [review]
patch v1.1

Added a new test harness we can use in bug 715133 and a test using it
Comment 6 User image :Gavin Sharp [email:] 2012-01-24 10:18:18 PST
Comment on attachment 591152 [details] [diff] [review]
patch v1.1

>diff --git a/toolkit/components/places/tests/inline/head_autocomplete.js b/toolkit/components/places/tests/inline/head_autocomplete.js

>+function run_test() {

>+    try {
>+      Services.prefs.clearUserPref("browser.urlbar.autoFill");
>+    } catch(ex) {}
>+    try {
>+      Services.prefs.clearUserPref("places.history.enabled");
>+    } catch(ex) {}

Don't need the try/catches anymore (since bug 487059)
Comment 7 User image Marco Bonardo [::mak] 2012-01-24 11:43:38 PST
Removed the try/catch
Comment 8 User image :Ms2ger (⌚ UTC+1/+2) 2012-01-25 07:26:29 PST

Note You need to log in before you can comment on or make changes to this bug.