Closed Bug 1432745 Opened 6 years ago Closed 6 years ago

Improve or remove the persistent status column in the site data manager

Categories

(Firefox :: Settings UI, enhancement, P1)

60 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: johannh, Assigned: johannh)

References

(Blocks 1 open bug)

Details

(Whiteboard: [storage-v2])

Attachments

(2 files)

The mockups for the new site data manager in bug 1421690 don't include a "status" column to indicate persistent storage, since we feel it's a lot of wasted space that is not used by most websites and can be pretty confusing to the user.

We should consider a different way of indicating the "persistent" status, maybe through a sort of optional label in the storage column (or simply adding (persistent) in parentheses).
Priority: P3 → P2
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: P2 → P1
Attached image screenshot.png
Jacqueline, this patch just adds (Persistent) to the storage usage column in the site data manager, if the site persisted the storage (which is quite rare). It looks like in the attachment. Let me know if that works for you.
Attachment #8952664 - Flags: ui-review?(jsavory)
Comment on attachment 8952661 [details]
Bug 1432745 - Merge the "persistent" column in the site data manager into the "usage" column.

https://reviewboard.mozilla.org/r/221906/#review228256

The localization notes are a bit fishy (see my comment below), but I won't let that block r+.

::: browser/components/preferences/siteDataSettings.js:59
(Diff revision 1)
>  
>      // Add "Storage" column
> -    if (site.usage > 0) {
> +    if (site.usage > 0 || site.persisted) {
>        let size = DownloadUtils.convertByteUnits(site.usage);
> -      let str = this._prefStrBundle.getFormattedString("siteUsage", size);
> -      addColumnItem(str, "1");
> +      let str;
> +      if (site.persisted) {

nit/suggestion: You could use the ternary operator for the string name to avoid the long phrase "this._prefStrBundle.getFormattedString" twice:

let strName = site.persisted ? "siteUsagePersistent" : "siteUsage";
str = this._prefStrBundle.getFormattedString(strName, size);

::: browser/locales/en-US/chrome/browser/preferences/preferences.properties:180
(Diff revision 1)
> -persistent=Persistent
> +#LOCALIZATION NOTE (siteUsage): This is the total usage of site data.
> +#   e.g., "200 MB"
> +#   %1$S = size
> +#   %2$S = unit (MB, KB, etc.)
>  siteUsage=%1$S %2$S
> +#LOCALIZATION NOTE (siteUsagePersistent): This is the total usage of site data.

Um, I don't understand this, the example doesn't seem to match the string?

And the note is otherwise the exact same as the one above, so we could avoid some duplication here and "merge" these localization notes.
Attachment #8952661 - Flags: review?(nhnt11) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/57a398ca3a4d
Merge the "persistent" column in the site data manager into the "usage" column. r=nhnt11
https://hg.mozilla.org/mozilla-central/rev/57a398ca3a4d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.