845484 - Story - Choose types of private data to clear in the options flyout

Status: VERIFIED FIXED

(Tracking Graveyard :: Metro Operations, defect, P2)

Description

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: [Mockup] Options_v6

We currently don't have this option available. 

Since we are not going to implement the clear permission per site for V1, we will need to have a global action for clearing permissions. 

This option should be placed after "Clear Private Data".

Comment 1

[Asa Dotzler [:asa]]

Yuan, is this meant for clearing just the website permissions like location and app cache or also for clearing Firefox settings like remembering passwords and blocking pop-ups?

Comment 2

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

In my understanding, site permissions should be everything that the users have granted privileges to a website via the information bar. So I think it should include remembering passwords, blocking pop-ups.

Comment 3

[Asa Dotzler [:asa]]

(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #2)
> In my understanding, site permissions should be everything that the users
> have granted privileges to a website via the information bar. So I think it
> should include remembering passwords, blocking pop-ups.

In the case of geolocation and app cache, the Firefox user is giving the website permission.  In the case of password management and pop-up blocking, the user is giving Firefox instructions that the web site knows nothing about.

So, in the first cases, users are making a deal with a website and in the second case, users are making a deal with Firefox. Those aren't really the same thing. That being said, I haven't thought much about how users see these things or whether or not the distinction matters.

Comment 4

[Matt Brubeck (:mbrubeck)]

(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #0)
> This option should be placed after "Clear Private Data".

Would it make sense to have "Site Preferences" be one of several options available to clear in the "Clear Private Data" prompt?  This would match both desktop and mobile Firefox, and would mean less clutter in the main Options flyout.


(In reply to Asa Dotzler [:asa] from comment #3)
> So, in the first cases, users are making a deal with a website and in the
> second case, users are making a deal with Firefox. Those aren't really the
> same thing. That being said, I haven't thought much about how users see
> these things or whether or not the distinction matters.

I think it makes sense to group all site-specific settings together.  Any categories we assign like "content permissions" versus "browser settings" are too close to the implementation level for users to reliably understand.

For comparison, both desktop Firefox and Firefox for Android have a "Site Preferences" checkbox in their "Clear private data" dialogs, and this checkbox includes such things as password manager and pop-up blocker prefs.  And the desktop Firefox "Page Info" dialog and about:permissions page group most of these settings under "Permissions".

Comment 5

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

(In reply to Matt Brubeck (:mbrubeck) from comment #4)
> (In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #0)
> > This option should be placed after "Clear Private Data".
> 
> Would it make sense to have "Site Preferences" be one of several options
> available to clear in the "Clear Private Data" prompt?  This would match
> both desktop and mobile Firefox, and would mean less clutter in the main
> Options flyout.
> 

Originally my thought on having "Clear History" on top-level is to give the users a quick access to one or two frequent actions, at the same time avoiding hierarchies. But since "Clear History" was changed to an action for clearing all data. It has become an advanced setting option, which will be used less frequently. In this case, I agree that adding a section to support all clearing options could provide more value.

I believe many people, including myself, have no idea of what "private data" really means. We need to add a description to let the users know that it's not a "clear-all" function, and the users can have options. That was the reason that I have never opened "Clear private data" on FX android.

In terms of UI, I think the options flyout still needs to be flat, no secondary flyout. Once the users tap "Clear private data", the types of private data will be added onto the page for the users to deselect. (Will need to update the mockup)


> 
> (In reply to Asa Dotzler [:asa] from comment #3)
> > So, in the first cases, users are making a deal with a website and in the
> > second case, users are making a deal with Firefox. Those aren't really the
> > same thing. That being said, I haven't thought much about how users see
> > these things or whether or not the distinction matters.
> 
> I think it makes sense to group all site-specific settings together.  Any
> categories we assign like "content permissions" versus "browser settings"
> are too close to the implementation level for users to reliably understand.
> 
> For comparison, both desktop Firefox and Firefox for Android have a "Site
> Preferences" checkbox in their "Clear private data" dialogs, and this
> checkbox includes such things as password manager and pop-up blocker prefs. 
> And the desktop Firefox "Page Info" dialog and about:permissions page group
> most of these settings under "Permissions".

Comment 7

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Attachment: [Mockup] Clear Private Data_v2

After the conversations we have had, I think we have agreed upon supporting clear different types of data, a consistent approach with Desktop and mobile. 

Notes on the design decisions:
1. I decided to promote "Clear Browsing History" on the top level, since it's generally the most commonly used action. And by doing that, the users don't have to deselect unwanted items.

2. In the category of "Other data(passwords, cache, cookies, etc)", I didn't include "Downloaded files". I assumed if we are letting the OS handle downloaded file management, we will not have to provide this option. Also, normally it's no natural relationship between "downloaded files" to "Private data".

3. I think it's for the users' best if we don't remember their last-time selections on the types of data.
When "Other data" is checked, the section should expand and show the secondary options in checked checkbox. It's the most predictable result when the we have the "Other data" section dismissed by default. 


Let me know if there is any question. Thanks!

Comment 8

[Jim Mathies [:jimm]]

general->flyouts, 3 bugs

Comment 10

[Marco Mucci [:MarcoM]]

Hi Asa, a new user story is required.  In the meantime, Rodrigo will use the existing comments and design attachments as a guide to start.

Comment 11

[Rodrigo Silveira [:rsilveira] [:rodms]]

Desktop has an "Active logins" option for clearing session history. Should we add this extra option for parity, combine this with some other option or just leave it out?

Comment 12

[Asa Dotzler [:asa]]

(In reply to Rodrigo Silveira [:rsilveira] from comment #11)
> Desktop has an "Active logins" option for clearing session history. Should
> we add this extra option for parity, combine this with some other option or
> just leave it out?

Yes, let's add active logins to the expanded section. Full story coming soon.

Comment 13

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Patch v1

Comment 14

[Matt Brubeck (:mbrubeck)]

Comment on attachment 729343 [details] [diff] [review]
Patch v1

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

Looks great!  See below for some style nits, mostly just to make the code a bit shorter in places.

::: browser/metro/base/content/browser-ui.js
@@ +1429,5 @@
>  var SyncPanelUI = {
>    init: function() {
>      // Run some setup code the first time the panel is shown.
>      Elements.syncFlyout.addEventListener("PopupChanged", function onShow(aEvent) {
> +      if (aEvent.detail && aEvent.popup && aEvent.popup.panel === Elements.syncFlyout) {

Can you use aEvent.target instead of aEvent.popup.panel here?

@@ +1700,3 @@
>      let event = document.createEvent("UIEvents");
>      event.initUIEvent("PopupChanged", true, true, window, aVisible);
>      event.popup = this._popup;

...and if we switch to using the event target, we can just get rid of the event.popup property here.

::: browser/metro/base/content/preferences.js
@@ +6,5 @@
>  var PreferencesPanelView = {
>    init: function pv_init() {
>      // Run some setup code the first time the panel is shown.
>      Elements.prefsFlyout.addEventListener("PopupChanged", function onShow(aEvent) {
> +      if (aEvent.detail && aEvent.popup && aEvent.popup.panel === Elements.prefsFlyout) {

...and this can also switch to aEvent.target.

::: browser/metro/base/content/sanitizeUI.js
@@ +1,4 @@
> +// -*- Mode: js2; tab-width: 2; indent-tabs-mode: nil; js2-basic-offset: 2; js2-skip-preprocessor-directives: t; -*-
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

I don't always remember it, but I'm trying to get into the habit of adding "use strict"; to all new JS files.

@@ +27,5 @@
> +    let clearNotificationDone = document.getElementById("clear-notification-done");
> +    let allCheckboxes = SanitizeUI._privData.querySelectorAll("checkbox");
> +    let allSelected = SanitizeUI._privData.querySelectorAll(
> +      "#prefs-privdata-history[checked], " +
> +      "#prefs-privdata-other[checked] + #prefs-privdata-subitems .privdata-subitem-item[checked]");

Can you use a simpler selector, like:

  let allSelected = this._privData.querySelectorAll("checkbox[itemName][checked]");

@@ +66,5 @@
> +  /* Disable the clear button when nothing is selected */
> +  _onCheckboxChange: function _onCheckboxChange() {
> +    let anySelected = SanitizeUI._privData.querySelector(
> +      "#prefs-privdata-history[checked], " +
> +      "#prefs-privdata-other[checked] + #prefs-privdata-subitems .privdata-subitem-item[checked]");

See selector sugestion above.

@@ +73,5 @@
> +    let clearButton = document.getElementById("prefs-clear-data");
> +    if (hasSelection) {
> +      clearButton.removeAttribute("disabled");
> +    } else {
> +      clearButton.setAttribute("disabled", "true");

You can use the .disabled property instead of setAttribute/removeAttribute:

clearButton.disabled = !anySelected;

::: browser/metro/locales/en-US/chrome/preferences.dtd
@@ +30,5 @@
> +<!ENTITY optionsHeader.privacy.clearPrivateData.sitePref         "Site preferences">
> +<!ENTITY optionsHeader.privacy.clearPrivateData.formSearchHist   "Form &amp; search history">
> +<!ENTITY optionsHeader.privacy.clearPrivateData.passwords        "Saved passwords">
> +<!ENTITY optionsHeader.privacy.clearPrivateData.offline          "Offline website data">
> +<!ENTITY optionsHeader.privacy.clearPrivateData.logins           "Active logins">

Minor naming nit: I don't like how long these entity names are getting.  Let's change them all from "optionsHeader.privacy.clearPrivateData.*" to just "clearPrivateData.*".

Comment 15

[Matt Brubeck (:mbrubeck)]

Comment on attachment 725664 [details]
[Mockup] Clear Private Data_v2

It might not be obvious that checking "[ ] Other data (passwords, cache, cookies, etc)" will give them a chance to choose options.  I worry that users won't check this box (it sounds scary) and won't discover the hidden checkboxes.

Is there any way we can indicate more clearly that further choices are available?  What if the checkbox had a label like "[ ] Choose other data (...)" or maybe if we used a "twisty" instead of a checkbox?

Comment 16

[Asa Dotzler [:asa]]

(In reply to Matt Brubeck (:mbrubeck) from comment #15)
> Comment on attachment 725664 [details]
> [Mockup] Clear Private Data_v2
> 
> It might not be obvious that checking "[ ] Other data (passwords, cache,
> cookies, etc)" will give them a chance to choose options.  I worry that
> users won't check this box (it sounds scary) and won't discover the hidden
> checkboxes.
> 
> Is there any way we can indicate more clearly that further choices are
> available?  What if the checkbox had a label like "[ ] Choose other data
> (...)" or maybe if we used a "twisty" instead of a checkbox?

Let's worry about that bit of trickiness later (what does Metro do for a disclosure widget?) We don't have to get this flow perfect, we just need something functional for v1.

Comment 17

[Rodrigo Silveira [:rsilveira] [:rodms]]

Attachment: Patch v2

(In reply to Matt Brubeck (:mbrubeck) from comment #14)

Thanks for the review! Updated the patch after the review. The only item I didn't do was the updated query for the checked items.

The sub-items of the others check box are all selected but are not visible. They should not appear in the query.

Comment 18

[Marco Mucci [:MarcoM]]

Hi Rodrigo, I spoke with Asa and he'll create the story after your story lands.  If there is anything missing in completed feature from what is outlined in the story then we'll just create a follow-up Change Story to address it.

Comment 19

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/077eff4dede3

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/077eff4dede3

Comment 21

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (Windows NT 6.2; rv:23.0) Gecko/20130405 Firefox/23.0

All options seem to be working, apart from Downloads and Active logins:
-the items in Downloads are still present after clearing Downloads
-logged in facebook - hit clear active logins -> I could navigate within facebook - I would have expected to be logged out.

If the two are expected/don't raise objections, then this can be set to verified.

FTR, the Clear button doesn't work sometimes - but after pressing any key, the button works again - which would make that the same issue as bug 856244.

Comment 22

[Marco Mucci [:MarcoM]]

Hi Asa, please see Comment #21 from Virgil.

Comment 23

[Rodrigo Silveira [:rsilveira] [:rodms]]

For the downloads to disappear I had to restart metro. Seems like it just needs a refresh. 

Thinking about it, I'm not sure what clear active logins is supposed to do. According to this support answer it's what Virgil described: https://support.mozilla.org/en-US/questions/939252?esab=a&s=clear+active+logins&r=0&as=s

But can you log out of facebook without clearing cookies? Anyway, I see the same behavior on desktop - nothing happens.

Comment 24

[Matt Brubeck (:mbrubeck)]

(In reply to Virgil Dicu [:virgil] [QA] from comment #21)
> All options seem to be working, apart from Downloads and Active logins:
> -the items in Downloads are still present after clearing Downloads

The download list is going away soon (to be replaced by bug 831942), so we might not bother fixing this in the meantime -- unless it's a simple fix to help us verify this bug.

> -logged in facebook - hit clear active logins -> I could navigate within
> facebook - I would have expected to be logged out.

"Active logins" refers to HTTP Auth sessions.  This is certainly confusing for users, because the vast majority of sites use cookies for login instead of HTTP Auth.  Maybe we should combine these into a single "Cookies and logins" checkbox?  Asa, what do you think?

Comment 25

[Asa Dotzler [:asa]]

(In reply to Matt Brubeck (:mbrubeck) from comment #24)
> (In reply to Virgil Dicu [:virgil] [QA] from comment #21)
> > All options seem to be working, apart from Downloads and Active logins:
> > -the items in Downloads are still present after clearing Downloads
> 
> The download list is going away soon (to be replaced by bug 831942), so we
> might not bother fixing this in the meantime -- unless it's a simple fix to
> help us verify this bug.
> 
> > -logged in facebook - hit clear active logins -> I could navigate within
> > facebook - I would have expected to be logged out.
> 
> "Active logins" refers to HTTP Auth sessions.  This is certainly confusing
> for users, because the vast majority of sites use cookies for login instead
> of HTTP Auth.  Maybe we should combine these into a single "Cookies and
> logins" checkbox?  Asa, what do you think?


We should fix this when Firefox desktop fixes this. For now, we should go for consistency.

Comment 26

[Matt Brubeck (:mbrubeck)]

(In reply to Asa Dotzler [:asa] from comment #25)
> > "Active logins" refers to HTTP Auth sessions.  This is certainly confusing
> > for users, because the vast majority of sites use cookies for login instead
> > of HTTP Auth.  Maybe we should combine these into a single "Cookies and
> > logins" checkbox?  Asa, what do you think?
> 
> We should fix this when Firefox desktop fixes this. For now, we should go
> for consistency.

Filed bug 860440 for changing this on desktop.  I'll file follow-ups for Metro and Android if the desktop UX team agrees to the change.

Comment 27

[Virgil Dicu [:virgil] [QA]]

Marking this as verified considering comments 23, 24, 25.

The Active logins issue is consistent with Firefox desktop behavior and I've filed a separate bug for the download issue: bug 861901

Comment 28

[Jim Mathies [:jimm]]

verified it7, including logging out of facebook. Bookmarks were not cleared, but we have a bug on this already which is up for discussion. Everything else was removed the from start page.

Comment 29

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130826074752
Built from http://hg.mozilla.org/mozilla-central/rev/14b1e8c2957e

Didn't WFM
Tested on windows 8 using latest nightly for iteration-12. Used Cler Private data, but bookmark didn't clear.
For this bug is already created. (Bug872234)