493527 - Custom prefs are not reset in privacy pane when selecting Remember history / Never remember history

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b5pre) Gecko/20090517 Shiretoko/3.5b5pre ID:20090517031745

With the patch on bug 490199 some custom prefs aren't reset when you choose "Remember history" or "Never remember history" from the dropdown. As result the pane always displays the custom settings. This is irritating and users cannot really see which of those prefs is causing this problem.

Steps:
1. Open preferences and select privacy pane
2. Change the days to another value as 90 days
3. Switch to Remember or Never remember dropdown option
4. Close preferences and reopen again

After step 4 the custom settings are shown whether which new selection the user made before closing the preferences pane.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

I'm having trouble understanding this bug report.

If a user selects custom settings, and changes things from the default (ex: days to 90 days), then selects "Remember History", then switches BACK to "Custom Settings" I would expect that the custom settings would return to how the user had set them before (ex: 90 days).

Is that not happening?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mike, all of those checkboxes are getting restored to their default values except the one for the textfield. It stays at the given input from the user.

Alex, how do we have to behave here correctly?

Comment 3

[Drew Willcoxon :adw]

(In reply to comment #1)
> Is that not happening?

That's not happening.  This is:

1. Selecting "Remember history" resets all custom prefs to defaults except the number of days to remember.
2. Selecting "Never remember history" doesn't reset any custom prefs.
3. All the fix to bug 490199 does is show the custom pane if any custom prefs are not default and one of the two simpler panes (depending on the boolean value of auto-start) otherwise.

We are thinking that all custom prefs should be reset for both points 1 and 2 above (bug 490199 comment 26 and 27).  That's not the behavior described in comment 1 though.

This isn't a regression BTW.  The patch mentioned in comment 0 didn't change anything about prefs being reset or not.

My head hurts.

Comment 4

[(no longer active)]

I think ideally on Windows, the behavior should be that of comment 1, but on Linux and Mac that of comment 3, since on those platforms prefs are applied immediately, but on Windows only when the OK button is pressed.

Waiting for a decision from a UI person, though.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Removing regression flag as per comment 3 (thanks, Drew)
This will not block the final release of Firefox 3.5

As for a UI decision: we should follow the behaviour in comment 1. Eshan rightly points out, however, in comment 4 that we should be careful to only use the settings when the user has actually set them - which is the minute they check/uncheck the boxes on OSX, or when they hit Apply/OK on Windows/Linux.

So, to make it a little clearer:

 - there are three choices (Always, Never, Custom)
 - when a user selects Custom and customizes the sub-settings and applies them (instant for OSX, on Apply/OK on w32/linux) those settings should be remembered and associated with "Custom"
 - if the user flips back to "Always", applies, and then back to "Custom" they should see the user's previously set custom settings

Removing ui-wanted per this comment (thanks, me)

Comment 6

[(no longer active)]

(In reply to comment #5)
>  - when a user selects Custom and customizes the sub-settings and applies them
> (instant for OSX, on Apply/OK on w32/linux) those settings should be remembered
> and associated with "Custom"

This should be (instant for OSX/linux, on Apply/OD on w32), right?

>  - if the user flips back to "Always", applies, and then back to "Custom" they
> should see the user's previously set custom settings

Only on Windows, correct?

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Correct, only when the preferences are applied, which is not instant on Windows.

Not blocking, but nice polish to have.

Comment 8

[maxim]

Attachment: 0001-DB-1173-History-preference-not-updated-when-selectin.patch

Hi guys.
I work at Cliqz, which you probably heard of, also using Firefox source for its product :)
We received complaints from our users about this issue recently, and fixed it in our repo.
If you like, here's the patch.
It may be imperfect in the sense of delayed pref changes policy for Windows, but that's anyway, how the rest of prefs around are treated already. And it should better that current behaviour.

Also, these reports seem to be all duplicates:
https://bugzilla.mozilla.org/show_bug.cgi?id=1228686
https://bugzilla.mozilla.org/show_bug.cgi?id=1255473
https://bugzilla.mozilla.org/show_bug.cgi?id=1298065

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8839853 [details] [diff] [review]
0001-DB-1173-History-preference-not-updated-when-selectin.patch

Jared, can you take a look at this?

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8839853 [details] [diff] [review]
0001-DB-1173-History-preference-not-updated-when-selectin.patch

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

Thanks for the patch. We don't have any delayed pref changes on Windows anymore, we switched away from that when we moved the prefs to inside of the Firefox window.

I can't find out why this was setting it through the checkbox but the others were set through the preference element. Looking in the history it looks like this was always written this way, from http://searchfox.org/mozilla-central/diff/432c74dd9ba6f4b6bba660ed99402ffa69dba85a/browser/components/preferences/privacy.js and it was added by bug 520165.

So, this looks fine. Can you update your commit message to (note, multiline):

"Bug 493527 - Directly change the remember-history pref through the preference element since setting it through the checkbox required multiple attempts. r=jaws

This looks to be happening because the binding for the checkbox may get torn down too fast when a restart is required."

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Maxim, are you going to update the commit message or should I?

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 493527 - Directly change the remember-history pref through the preference element since setting it through the checkbox required multiple attempts.

This looks to be happening because the binding for the checkbox may get torn down too fast when a restart is required.

Review commit: https://reviewboard.mozilla.org/r/123174/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/123174/

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8850624 [details]
Bug 493527 - Directly change the remember-history pref through the preference element since setting it through the checkbox required multiple attempts.

https://reviewboard.mozilla.org/r/123174/#review125556

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ab9adff3faf
Directly change the remember-history pref through the preference element since setting it through the checkbox required multiple attempts. r=jaws

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0ab9adff3faf