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

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Preferences
RESOLVED FIXED
8 years ago
3 months ago

People

(Reporter: whimboo, Assigned: maxim)

Tracking

({polish})

3.5 Branch
Firefox 55
polish
Points:
---
Bug Flags:
blocking-firefox3.5 -
blocking-firefox3.6 -
wanted-firefox3.5 +
wanted-firefox3.6 +

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
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.
Flags: blocking-firefox3.5?
Assignee: nobody → ehsan.akhgari
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?
(Reporter)

Comment 2

8 years ago
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?
Keywords: uiwanted

Comment 3

8 years ago
(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.
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.
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)
Flags: wanted-firefox3.5+
Flags: blocking-firefox3.6?
Flags: blocking-firefox3.5?
Flags: blocking-firefox3.5-
Keywords: regression, uiwanted
(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?
(Reporter)

Updated

8 years ago
Keywords: uiwanted
Correct, only when the preferences are applied, which is not instant on Windows.

Not blocking, but nice polish to have.
Flags: wanted-firefox3.6+
Flags: blocking-firefox3.6?
Flags: blocking-firefox3.6-
Keywords: uiwanted → polish
(Assignee)

Comment 8

4 months ago
Created attachment 8839853 [details] [diff] [review]
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 on attachment 8839853 [details] [diff] [review]
0001-DB-1173-History-preference-not-updated-when-selectin.patch

Jared, can you take a look at this?
Attachment #8839853 - Flags: review?(jaws)
Assignee: ehsan → maxim
Status: NEW → ASSIGNED
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."
Attachment #8839853 - Flags: review?(jaws) → review+
Maxim, are you going to update the commit message or should I?
Flags: needinfo?(maxim)
Flags: needinfo?(maxim)
Comment hidden (mozreview-request)

Comment 13

3 months ago
mozreview-review
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
Attachment #8850624 - Flags: review?(jaws) → review+

Comment 14

3 months ago
Pushed by jwein@mozilla.com:
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

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0ab9adff3faf
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.