1312372 - List sites using storage in Settings of Site Data

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Fischer [:Fischer]]

The sites which has requested persistent-storage permission should be listed in Settings of Site Data. 
The listed info including:
- Site host
- permission status
- storage usage

Comment 1

[Fischer [:Fischer]]

Attachment: Bug 1312372 - List sites using storage in Settings of Site Data,

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

Comment 2

[Fischer [:Fischer]]

Attachment: bug_1312372 _list_sites.png

Comment 3

[Fischer [:Fischer]]

Comment on attachment 8814802 [details]
Bug 1312372 - List sites using storage in Settings of Site Data,

@Jaws,

This bug lists site in the Settings dialog of Site Data.
Please see the attachment 8814804 [details]: bug_1312372 _list_sites.png

The other of functions in the spec[1] would be implemented in other following bugs one by one.

Thanks

[1] https://mozilla.invisionapp.com/share/4Y87EJO39#/screens/179637924

Comment 4

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

Comment on attachment 8814802 [details]
Bug 1312372 - List sites using storage in Settings of Site Data,

https://reviewboard.mozilla.org/r/95896/#review98720

Looks good, r=me with the following changes.

::: browser/components/preferences/siteDataSettings.js:61
(Diff revision 1)
> +      let size = DownloadUtils.convertByteUnits(data.usage);
> +      let item = document.createElement("richlistitem");
> +      item.setAttribute("data-origin", data.uri.spec);
> +      item.setAttribute("host", data.uri.host);
> +      item.setAttribute("status", prefStrBundle.getString(statusStrId));
> +      item.setAttribute("usage", `${size[0]} ${size[1]}`);

The usage string should not be constructed manually here. Follow the code at http://searchfox.org/mozilla-central/source/browser/components/places/content/places.js#431-433 for how this should be done.

::: browser/locales/en-US/chrome/browser/preferences/siteDataSettings.dtd:6
(Diff revision 1)
> +<!-- 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/. -->
> +
> +<!ENTITY     window.title                  "Settings - Site Data">
> +<!ENTITY     settings.description          "The following websites asked to store site data in your disk. You can specify which websites are allowed to store site data. Default site data is temporary and could be deleted automatically.">

We should rephrase this a little to make it clearer. Are we showing all sites that requested to store data or only sites that were granted?

"Below is a list of websites storing data on your disk. By default, this data is temporary and may be deleted automatically."

We can leave out the part about "you can specify which websites are allowed to store site data" since the Remove button will make that obvious.

Comment 5

[Fischer [:Fischer]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> ::: browser/components/preferences/siteDataSettings.js:61
> (Diff revision 1)
> > +      item.setAttribute("usage", `${size[0]} ${size[1]}`);
> 
> The usage string should not be constructed manually here. Follow the code at
> http://searchfox.org/mozilla-central/source/browser/components/places/
> content/places.js#431-433 for how this should be done.
> 
Thanks. Will update to use `getFormattedString` for formating string.
> ::: browser/locales/en-US/chrome/browser/preferences/siteDataSettings.dtd:6
> (Diff revision 1)
> > +<!ENTITY     settings.description          "The following websites asked to store site data in your disk. You can specify which websites are allowed to store site data. Default site data is temporary and could be deleted automatically.">
> 
> We should rephrase this a little to make it clearer. Are we showing all
> sites that requested to store data or only sites that were granted?
We are showing all sites that requested to store persistent data before disregarding granted or not-granted
> 
> "Below is a list of websites storing data on your disk. By default, this
> data is temporary and may be deleted automatically."
> 
> We can leave out the part about "you can specify which websites are allowed
> to store site data" since the Remove button will make that obvious.
The current strings are still under review. Because the review schedule is uncertain, we implement functions first. Then update strings if required later. Thanks for pointing out this, will have UX look at this.

Comment 6

[Fischer [:Fischer]]

Hi Tina,
Would you please look at the comment 4 while reviewing the strings? There is the string suggestion which can make the description more concise and clearer. Thanks.

Comment 7

[Fischer [:Fischer]]

Comment on attachment 8814802 [details]
Bug 1312372 - List sites using storage in Settings of Site Data,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/95896/diff/1-2/

Comment 8

[Fischer [:Fischer]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9848fcab26d4e1d725964fa39b0367d815715f95

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74f4edb0d09d
List sites using storage in Settings of Site Data, r=jaws

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/74f4edb0d09d