Closed Bug 1442850 Opened 6 years ago Closed 6 years ago

Changing the cookies settings also changes "Firefox will" to "Use custom settings for history"

Categories

(Firefox :: Settings UI, defect, P1)

Product:
Firefox
Component:
Settings UI
Version:
60 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 60
Tracking Flags:
Tracking Status
firefox59 --- unaffected
firefox60 --- verified

People

(Reporter: mstanke, Assigned: johannh)

References

Details

(Keywords: nightly-community, Whiteboard: [storage-v2][triage])

Attachments

(1 file)

Bug 1442850 - Update how history mode and cookie lifetime controls interact to fit our new preferences UI.
59 bytes, text/x-review-board-request
Gijs
: review+
Details 
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 ID:20180302220122

STR:
1) Open about:preferences#privacy
2) Verify "Firefox will" is set to default "Remember history".
3) Find cookies settings.
4) Change e.g. 3rd party cookies settings to "from visited".
5) Close Preferences.
6) Open about:preferences#privacy again.
7) See the "Firefox will" setting is now set to "Use custom settings for history".

It also works vice versa - changing "Firefox will" to default "Remember history" resets all cookies settings after you reopen the Preferences page.

This is probably a relict from the time where the cookies settings were underneath the history preference. Not sure how severe this is, but I would fix it ASAP for Firefox 60 to avoid any confusion for users.
Johann, who's the UX person for this project and have they considered this?
Flags: needinfo?(jhofmann)
(In reply to :Gijs from comment #1)
> Johann, who's the UX person for this project and have they considered this?

I'll check with jsavory but personally I think this is just a bug (I didn't try reloading the page) and I'll fix it...
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)
Priority: -- → P1
Whiteboard: [storage-v2][triage]
(In reply to Johann Hofmann [:johannh] from comment #2)
> (In reply to :Gijs from comment #1)
> > Johann, who's the UX person for this project and have they considered this?
> 
> I'll check with jsavory but personally I think this is just a bug (I didn't
> try reloading the page) and I'll fix it...

Which of these behaviours is a bug? Personally, I think that if you change the cookie setting, 'use custom settings' is the expected value for history. Though obviously, changing the history setting back to 'remember history' shouldn't obliterate your cookie settings without telling you.
The thing is after bug 1432743, cookies are no longer in the history section. So changing the cookies/history settings does not make much sense to me to change also the history/cookies settings, which are elsewhere.

If it's desired to change history settings when you decide e.g. to block 3rd party cookies, it should at least happen in the preferences UI immediately, not after a reload. However as you pointed out :Gijs, then it would work only a one way, but not back.
I think the history section should just be completely independent of the new cookies and site data section. I can see that "use custom settings" makes sense if you have the context, but I don't think two visually separate pieces of preferences should affect each other that way (there's also no guarantee they will always live that close to each other).
Comment on attachment 8956127 [details]
Bug 1442850 - Update how history mode and cookie lifetime controls interact to fit our new preferences UI.

https://reviewboard.mozilla.org/r/225060/#review231246

::: browser/components/preferences/in-content/privacy.js:629
(Diff revision 1)
>     * Update the privacy micro-management controls based on the
>     * value of the private browsing auto-start checkbox.

Please update this comment while we're here - it's not a checkbox?

::: browser/components/preferences/in-content/privacy.js:636
(Diff revision 1)
>     */
>    updatePrivacyMicroControls() {
> +    // Set "Keep cookies until..." to "I close Nightly" and disable the setting
> +    // when we're in auto private mode (or reset it back otherwise).
> +    document.getElementById("keepCookiesUntil").value = this.readKeepCookiesUntil();
> +    this.readAcceptCookies();

So AIUI this will disable the cookie control when permanent pb mode is on. But that's not going to be obvious in the site data section. Should we have some text there that explains the cookie 'keep until' menu is disabled because of permanent private browsing mode?
Attachment #8956127 - Flags: review?(gijskruitbosch+bugs)
Blocks: 1443644
Comment on attachment 8956127 [details]
Bug 1442850 - Update how history mode and cookie lifetime controls interact to fit our new preferences UI.

https://reviewboard.mozilla.org/r/225060/#review231246

> Please update this comment while we're here - it's not a checkbox?

It is a checkbox if you set "Use custom settings for history", but I can make that "preference" so that it covers both...

> So AIUI this will disable the cookie control when permanent pb mode is on. But that's not going to be obvious in the site data section. Should we have some text there that explains the cookie 'keep until' menu is disabled because of permanent private browsing mode?

As discussed on IRC, we will fix this in follow-up bug 1442850 because I'd like to avoid blocking this bug fix on UX and new strings.
Comment on attachment 8956127 [details]
Bug 1442850 - Update how history mode and cookie lifetime controls interact to fit our new preferences UI.

https://reviewboard.mozilla.org/r/225060/#review231436

rs=me
Attachment #8956127 - Flags: review?(gijskruitbosch+bugs) → review+
Thanks!
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f1e47fffcf5d
Update how history mode and cookie lifetime controls interact to fit our new preferences UI. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/f1e47fffcf5d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Works for me in Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 ID:20180307220100.
Status: RESOLVED → VERIFIED
status-firefox59: --- → unaffected
status-firefox60: fixed → verified
Depends on: 1457180
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: