249610 - disabling form autocomplete doesn't take effect until restart

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Tracy Walker [:tracy]]

seen on windows build 2004-07-02-11-0.9

1. Type a search topic in the Search Bar, hit Enter.
2. Type the first character of the same topic in the Search Bar.  Notice the
Drop Down with the full topic ready to be selected to fill the search field.
3. Goto Tools | Options | Privacy
4. Under Saved Form Information uncheck the setting for Save information I enter
in web page forms and the Search Bar.
5. Type the first character of the same topic in the Search Bar.

tested results:
The drop down appears with the full topic.

expected results:
No drop down appears. the suer must complete the topic manually

6. Restart the app.
7. Type the first character of the same topic in the Search Bar.

results:  now the form completion is off as selected in the prefs before
restart.  a drop down does not appear.  This is as expected.

I've also seen this non-dynamic updating of a pref setting with Tools | Options
| Advanced | Browsing | Hide the tab bar when only one web site is open.

change the setting for that pref but it doesn't take the new state until a
restart of the app.

Comment 1

[Steffen Wilberg]

I can confirm that enabling/disabling form history doesn't take effect until
restarting Firefox.
Note that switching the pref doesn't clear the form history. If you enable it
(and restart Firefox), it's available again.

The "hide tab bar" issue is bug 177506.
-> limiting summary to the first issue.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040707
Firefox/0.9.0+ (Steffen)

Comment 2

[Tracy Walker [:tracy]]

Marcia just discovered that the pref for allowing cookies for the current
session only doesn't take until restart

Comment 3

[Steffen Wilberg]

Mike, see comment 2.

Comment 4

[Mike Connor [:mconnor]]

the pref for allowing for current session only shouldn't have that problem, at
all.  If it does, its a backend problem that would affect Mozilla 1.7.x, 1.8x,
and all trunk and branch builds since the 1.7b timeframe.

Steffen, I'm the QA contact for this bug, you don't need to cc me ;)

Comment 5

[Marcia Knous [:marcia]]

Using Build ID: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7)
Gecko/20040715 Firefox/0.9.1+, unchecking the Save Form information pref box did
not work properly until I restarted the browser.

Comment 6

[Mike Connor [:mconnor]]

(In reply to comment #0)
> I've also seen this non-dynamic updating of a pref setting with Tools | Options
> | Advanced | Browsing | Hide the tab bar when only one web site is open.

that's bug 177506.  And it does actually take effect without a restart, it just
doesn't update the UI immediately.

Start with it checked, make sure you have one tab open.  Open a new tab, the tab
bar appears.  Close the second tab, the tab bar stays.

I'm going to change this into a bug for a single issue.

If the cookie acceptance doesn't change on Firefox, it will most likely be able
to be replicated on Mozilla builds of the same vintage.  That code does use
pref-observers and I've tested that code extensively (I rewrote that part of the
code during the 1.7 cycle).  Please file a bug under Browser->Cookies if this
can be replicated.

Comment 7

[Tracy Walker [:tracy]]

Okay, no problem, we'll write bugs for each occurance in the failure of an
individual pref to dynamically update. 

Leave this one for the form autocomplete pref not taking until restart.

Comment 8

[Blake Ross]

UI that seems broken is bad in a finished product. We should either fix it to
work without a restart (best case) or add text indicating that a restart is
necessary.

Comment 9

[Blake Ross]

-> bryner

Comment 10

[Brian Ryner (not reading)]

Attachment: patch

Comment 11

[Brian Ryner (not reading)]

Comment on attachment 154269 [details] [diff] [review]
patch

Actually I don't like the attachFormFill/detachFormFill part of this, it should
use the autoCompleteDisabled attribute instead.  New patch in a bit.

Comment 12

[Brian Ryner (not reading)]

Attachment: patch #2

This seems a little cleaner to me, in that disabling autocomplete is done the
same way for the search bar and for the form fields.

Comment 13

[Brian Ryner (not reading)]

Comment on attachment 154270 [details] [diff] [review]
patch #2

Just a little explanation of what I'm doing:

>@@ -1166,6 +1166,7 @@ function delayedStartup()
>   gFormFillPrefListener = new FormFillPrefListener();
>   var pbi = gPrefService.QueryInterface(Components.interfaces.nsIPrefBranchInternal);
>   pbi.addObserver(gFormFillPrefListener.domain, gFormFillPrefListener, false);
>+  gFormFillPrefListener.toggleFormFill();

This makes sure we set disableautocomplete on the search bar at startup if
necessary, which we currently don't.

>@@ -1347,6 +1348,8 @@ FormFillPrefListener.prototype =
>     catch (e) {
>     }
>     gBrowser.setAttribute("autocompleteenabled", gFormFillEnabled);
>+    var formController = Components.classes["@mozilla.org/satchel/form-fill-controller;1"].getService(Components.interfaces.nsIAutoCompleteInput);
>+    formController.disableAutoComplete = !gFormFillEnabled;
> 

This syncs the AutocompleteInput for form fill with the pref.

>--- toolkit/components/autocomplete/src/nsAutoCompleteController.cpp	21 May 2004 09:38:17 -0000	1.15.2.1.2.1
>+++ toolkit/components/autocomplete/src/nsAutoCompleteController.cpp	25 Jul 2004 08:28:49 -0000
>@@ -174,7 +174,7 @@ nsAutoCompleteController::HandleText()
> 
>   PRBool disabled;
>   mInput->GetDisableAutoComplete(&disabled);
>-  NS_ENSURE_TRUE(!disabled, NS_OK;);
>+  NS_ENSURE_TRUE(!disabled, NS_OK);
> 

typo fix.

>   nsAutoString newValue;
>   mInput->GetTextValue(newValue);
>@@ -269,6 +269,10 @@ nsAutoCompleteController::HandleKeyNavig
>   mInput->GetPopup(getter_AddRefs(popup));
>   NS_ENSURE_TRUE(popup != nsnull, NS_ERROR_FAILURE);
> 
>+  PRBool disabled;
>+  mInput->GetDisableAutoComplete(&disabled);
>+  NS_ENSURE_TRUE(!disabled, NS_OK);
>+

Make sure we don't show anything when you hit down arrow if autocomplete is
disabled, even if we've previously found results.

Comment 14

[Brian Ryner (not reading)]

Hm, I found a series of steps where autocomplete doesn't work after being
enabled on the fly, with this patch:

1. start up with autocomplete turned off
2. navigate to a page with a form input that you have previously saved
autocomplete results for
3. open a new tab
4. open prefs and enable form autocomplete, close prefs
5. switch back to the original tab

results will not autocomplete for the form input. hopefully i can debug that
tomorrow.

Comment 15

[Brian Ryner (not reading)]

Oops, that's not quite right.  Here are the steps (mostly for my reference):

1. start with with form fill disabled
2. open a new tab
3. load a page in the tab that has a form input you have saved values for
4. switch back to the first tab
5. open prefs, enable autocomplete, close prefs
6. switch back to the second tab

autocomplete will not function for the form input.

Comment 16

[Brian Ryner (not reading)]

Attachment: patch #3

The problem I mentioned was caused by attachFormFill() only being called if
autocomplete is enabled when the browser instance (tab) is created.  I thought
about this some and decided that this whole per-<browser> autocompleteenabled
business isn't really needed.  It's a lot less error-prone if we just always
hook up the form fill listener, and rely on the disableAutoComplete property on
the AutoCompleteInput to tell the autocomplete code to ignore input if the pref
is turned off.

Comment 17

[Brian Ryner (not reading)]

checked in

Comment 18

[Tracy Walker [:tracy]]

verified with 2004-08-02-08-0.9+