Closed Bug 1312372 Opened 3 years ago Closed 3 years ago

List sites using storage in Settings of Site Data

Categories

(Firefox :: Preferences, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: Fischer, Assigned: Fischer)

References

Details

(Whiteboard: [storage-v1])

Attachments

(2 files)

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
No longer depends on: 1312361
Blocks: 1312374
Blocks: 1312375
Blocks: 1312377
Blocks: 1312380
Depends on: 1313003
Depends on: 1313602
No longer depends on: 1313602
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
Attachment #8814802 - Flags: review?(jaws)
Assignee: nobody → fliu
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.
Attachment #8814802 - Flags: review?(jaws) → review+
(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.
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.
Flags: needinfo?(thsieh)
Blocks: 1323391
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74f4edb0d09d
List sites using storage in Settings of Site Data, r=jaws
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/74f4edb0d09d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Blocks: 1331318
No longer blocks: 1331318
Blocks: 1338036
No longer blocks: 1338036
Blocks: 1340967
No longer blocks: 1340967
Blocks: 1343477
Flags: needinfo?(thsieh)
You need to log in before you can comment on or make changes to this bug.