Last Comment Bug 740076 - autoFill should be disabled by the autocomplete preference
: autoFill should be disabled by the autocomplete preference
Status: VERIFIED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 14 Branch
: All All
: -- normal (vote)
: Firefox 15
Assigned To: Marco Bonardo [::mak]
:
:
Mentors:
Depends on:
Blocks: 735187 746572 751709
  Show dependency treegraph
 
Reported: 2012-03-28 11:37 PDT by Virtual_ManPL [:Virtual] - (ni? me)
Modified: 2012-05-10 05:56 PDT (History)
8 users (show)
mak77: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
Location Bar.png (6.36 KB, image/png)
2012-03-28 11:38 PDT, Virtual_ManPL [:Virtual] - (ni? me)
no flags Details
Options (29.81 KB, image/png)
2012-03-28 11:38 PDT, Virtual_ManPL [:Virtual] - (ni? me)
no flags Details
Important Modified Preferences (22.11 KB, image/png)
2012-03-28 11:39 PDT, Virtual_ManPL [:Virtual] - (ni? me)
no flags Details
patch v1.0 (6.45 KB, patch)
2012-05-04 03:36 PDT, Marco Bonardo [::mak]
dietrich: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Virtual_ManPL [:Virtual] - (ni? me) 2012-03-28 11:37:43 PDT
Location bar suggest some sites from bookmarks, even if all settings are disabled for autofilling in options...
Comment 1 Virtual_ManPL [:Virtual] - (ni? me) 2012-03-28 11:38:15 PDT
Created attachment 610219 [details]
Location Bar.png
Comment 2 Virtual_ManPL [:Virtual] - (ni? me) 2012-03-28 11:38:34 PDT
Created attachment 610220 [details]
Options
Comment 3 Virtual_ManPL [:Virtual] - (ni? me) 2012-03-28 11:39:12 PDT
Created attachment 610221 [details]
Important Modified Preferences
Comment 4 Tim (fmdeveloper) 2012-03-28 13:11:33 PDT
Confirmed on Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120328 Firefox/14.0a1 ID:20120328031211
Comment 5 Virtual_ManPL [:Virtual] - (ni? me) 2012-04-05 01:38:40 PDT
FYI - This is regression from version 13. When I will have more free time I will try to find it.
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-04-09 13:17:27 PDT
I imagine this is because the inline autocomplete search doesn't obey the "autocomplete" prefs.
Comment 7 Marco Bonardo [::mak] 2012-04-10 07:18:14 PDT
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #6)
> I imagine this is because the inline autocomplete search doesn't obey the
> "autocomplete" prefs.

right.
Comment 8 Virtual_ManPL [:Virtual] - (ni? me) 2012-04-27 02:59:28 PDT
BUMP, Will it make into Firefox 13 ?
I'm asking, because Firefox 12 was released and this bug is hella annoying as I always use location bar for google search and this issue make it completely unavailable ;p
So please, don't forget about this.
Comment 9 Alex Keybl [:akeybl] 2012-05-02 14:54:18 PDT
(In reply to Virtual_ManPL from comment #8)
> BUMP, Will it make into Firefox 13 ?

Inline auto-complete won't ship until FF14, so tracking for that release.
Comment 10 Alex Keybl [:akeybl] 2012-05-02 14:56:41 PDT
mak, would you be the right person to look into this for FF14, or would ddahl be?
Comment 11 Marco Bonardo [::mak] 2012-05-02 15:04:35 PDT
I may look at it, if we consider it a blocker for enabling the feature, currently we have no UI to enable/disable the feature, the prefs UI above controls the popup behavior, though that's completely unclear to the end user.
Comment 12 Marco Bonardo [::mak] 2012-05-02 16:24:04 PDT
I will split out some complication from here.

Basically, the blocking issue is that having autocomplete UI pref set to "Suggest nothing" actually is confusing, cause autoFill keeps suggesting urls. This should be easy to patch and test, and safe to backport to Aurora.

The issue on top, that I'd move to a separate bug, is that autoFill doesn't obey when selecting only-history or only-bookmarks options.  This is quite more complicated to get right, especially from a performances point of view. Since these are more technical options and autoFill acts already is some sort of exotic way (looking detached from the popup results and only filling typed entries) this issue looks far less problematic to live with.
Comment 13 Marco Bonardo [::mak] 2012-05-02 16:26:27 PDT
And as a side note to issue 2, we also have bug 530209, that suggests we want to revise the ui there, to clarify it.
Comment 14 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-05-03 10:58:18 PDT
(In reply to Marco Bonardo [:mak] from comment #12)

I concur wholeheartedly!
Comment 15 Marco Bonardo [::mak] 2012-05-04 03:36:14 PDT
Created attachment 621001 [details] [diff] [review]
patch v1.0
Comment 16 Dietrich Ayala (:dietrich) 2012-05-04 13:38:45 PDT
Comment on attachment 621001 [details] [diff] [review]
patch v1.0

Review of attachment 621001 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/tests/inline/test_autocomplete_functional.js
@@ +34,5 @@
>    "vis",
>    "visit2.mozilla.org/",
>    function ()
>    {
> +    Services.prefs.setBoolPref("browser.urlbar.autoFill", true);

optional fix: i wish this could be done in the right test, as a cleanup func, instead of spread across to an unrelated test.
Comment 17 Marco Bonardo [::mak] 2012-05-05 04:45:59 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #16)
> optional fix: i wish this could be done in the right test, as a cleanup
> func, instead of spread across to an unrelated test.

will move it to the head, so that each test starts in a clean env with known prefs setup.
Comment 19 Ryan VanderMeulen [:RyanVM] 2012-05-05 20:29:53 PDT
https://hg.mozilla.org/mozilla-central/rev/00c018e8dcef
Comment 20 Virtual_ManPL [:Virtual] - (ni? me) 2012-05-06 12:03:28 PDT
Verified with latest hourly.
Works properly now.
Thank you very much!
Comment 21 Marco Bonardo [::mak] 2012-05-07 12:51:00 PDT
Comment on attachment 621001 [details] [diff] [review]
patch v1.0

[Approval Request Comment]
Regression caused by (bug #): No regression, this is a blocker for keeping the feature enabled in Firefox 14
User impact if declined: Disable the feature
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): quite low, small patch leveraging existing code paths
String changes made by this patch: none
Comment 22 Alex Keybl [:akeybl] 2012-05-09 16:19:57 PDT
Comment on attachment 621001 [details] [diff] [review]
patch v1.0

[Triage Comment]
Approved for Aurora 14 since this will partially dictate whether the feature is left enabled on that release.

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