Obsolete UI for IndexedDB in Page Info/Permissions

RESOLVED FIXED in Firefox 58

Status

()

P3
normal
RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: yuki, Assigned: prathiksha)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 wontfix, firefox55 wontfix, firefox56 wontfix, firefox57 wontfix, firefox58 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Description:
At Firefox 43 (by the bug 1147821), individual permissions for IndexedDB became obsolete and it is controlled under the permission for Cookies.

Web Storage, IndexedDB, Cache API now obey third-party cookies preference
https://www.fxsitecompat.com/en-CA/docs/2015/web-storage-indexeddb-cache-api-now-obey-third-party-cookies-preference/

However, the "Page Info" dialog still has "Maintain Offline Storage => Use Default / Always Ask / Allow / Block" permission UI. Now those checkbox and radio buttons don't work and they should be go away.

Steps to reproduce:
1. Open "Page Info".
2. Choose "Permissions".

Expected:
There is no checkbox and radio buttons like "Use Default / Always Ask / Allow / Block" under the "Maintain Offline Storage" section.

Actual:
There are needless "Use Default / Always Ask / Allow / Block" checkbox and radio buttons. Nothing happens even if I touch those UI elements. (Note: the permission manager actually stores permissions for "indexedDB" to the database (permissions.sqlite), but no one respects the stored permission setting except the "Page Info" dialog itself.)
status-firefox54: affected → wontfix
status-firefox55: --- → wontfix
status-firefox56: --- → wontfix
status-firefox57: --- → wontfix
status-firefox58: --- → fix-optional
Priority: -- → P3
See Also: → bug 1400679
Blocks: 1208545
(Assignee)

Updated

a year ago
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8915197 [details]
Bug 1334411 - Remove the UI for indexedDB in Permissions section of PageInfo and the identity block.

https://reviewboard.mozilla.org/r/186434/#review191820

This looks mostly good, but please hide the indexedDB row when it's empty (or, to follow the existing pattern, hide it initially and show it in onIndexedDBUsageCallback).

There are probably tests for this that need to be removed as well, I'd suggest to do a try run.

::: browser/base/content/pageinfo/permissions.js:75
(Diff revision 1)
>      initPluginsRow();
>      return;
>    }
>  
> +  if (aPartId == "indexedDB") {
> +    createIndexedDBRow();

Nit: You could do this as part of initIndexedDBRow
Attachment #8915197 - Flags: review?(jhofmann)

Comment 3

a year ago
mozreview-review-reply
Comment on attachment 8915197 [details]
Bug 1334411 - Remove the UI for indexedDB in Permissions section of PageInfo and the identity block.

https://reviewboard.mozilla.org/r/186434/#review191820

> Nit: You could do this as part of initIndexedDBRow

Ah no you can't, nevermind me.
Comment hidden (mozreview-request)

Comment 5

a year ago
mozreview-review
Comment on attachment 8915197 [details]
Bug 1334411 - Remove the UI for indexedDB in Permissions section of PageInfo and the identity block.

https://reviewboard.mozilla.org/r/186434/#review193088

Sorry for the delay, I needed to make up my mind about this. I think it is best to remove this section entirely and make a new bug for finding a better place to put the information about stored site data in the page info window. There are many ways to clear indexedDB data in the browser and this one is just badly placed and confusing. We're planning to work on improving the UX around that in the coming months.
Attachment #8915197 - Flags: review?(jhofmann)
Duplicate of this bug: 1077986
Blocks: 1348223

Updated

a year ago
Blocks: 1354500
Comment hidden (mozreview-request)

Comment 8

a year ago
mozreview-review
Comment on attachment 8915197 [details]
Bug 1334411 - Remove the UI for indexedDB in Permissions section of PageInfo and the identity block.

https://reviewboard.mozilla.org/r/186434/#review197554

::: browser/base/content/pageinfo/permissions.js:10
(Diff revision 3)
>  /* import-globals-from pageInfo.js */
>  
>  Components.utils.import("resource:///modules/SitePermissions.jsm");
>  Components.utils.import("resource://gre/modules/BrowserUtils.jsm");
>  
>  const nsIQuotaManagerService = Components.interfaces.nsIQuotaManagerService;

I believe you can remove this as well.

::: browser/themes/shared/notification-icons.inc.css
(Diff revision 3)
>  .indexedDB-icon {
>    list-style-image: url(chrome://browser/skin/notification-icons/indexedDB.svg);
>  }
>  
> -.indexedDB-icon.blocked-permission-icon {
> -  list-style-image: url(chrome://browser/skin/notification-icons/indexedDB-blocked.svg);

I believe you can remove this file as well, now.
Attachment #8915197 - Flags: review?(jhofmann)
Comment hidden (mozreview-request)

Comment 10

a year ago
mozreview-review
Comment on attachment 8915197 [details]
Bug 1334411 - Remove the UI for indexedDB in Permissions section of PageInfo and the identity block.

https://reviewboard.mozilla.org/r/186434/#review197708

This looks good to me, but please also remove the indexedDB string, I don't think there's a way to access it anymore: https://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/locales/en-US/chrome/browser/sitePermissions.properties#38

(The only way to access it was here: https://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/browser/modules/SitePermissions.jsm#485)

Thank you!
Attachment #8915197 - Flags: review?(jhofmann) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 12

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4f223f11a08f
Remove the UI for indexedDB in Permissions section of PageInfo and the identity block. r=johannh
Keywords: checkin-needed
Backed out for failing xpcshell's browser/modules/test/unit/test_SitePermissions.js:

https://hg.mozilla.org/integration/autoland/rev/c4403a6818cb40608ae9e5c3f4af6891e728c7e9

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4f223f11a08ff2c3c6cefa68722a10113c04317c&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=139300172&repo=autoland

14:40:07     INFO -  TEST-START | browser/modules/test/unit/test_SitePermissions.js
14:40:07  WARNING -  TEST-UNEXPECTED-FAIL | browser/modules/test/unit/test_SitePermissions.js | xpcshell return code: 0
14:40:07     INFO -  TEST-INFO took 176ms
14:40:07     INFO -  >>>>>>>
14:40:07     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
14:40:07     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
14:40:07     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
14:40:07     INFO -  running event loop
14:40:07     INFO -  browser/modules/test/unit/test_SitePermissions.js | Starting testPermissionsListing
14:40:07     INFO -  (xpcshell/head.js) | test testPermissionsListing pending (2)
14:40:07  WARNING -  TEST-UNEXPECTED-FAIL | browser/modules/test/unit/test_SitePermissions.js | testPermissionsListing - [testPermissionsListing : 20] Correct list of all permissions - ["camera","cookie","desktop-notification","focus-tab-by-prompt","geo","image","install","microphone","persistent-storage","popup deepEqual ["camera","cookie","desktop-notification","focus-tab-by-prompt","geo","image","indexedDB","install","microphone","persistent-sto
14:40:07     INFO -  /Users/cltbld/tasks/task_1508879935/build/tests/xpcshell/tests/browser/modules/test/unit/test_SitePermissions.js:testPermissionsListing:20
14:40:07     INFO -  exiting test
14:40:07     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
14:40:07     INFO -  Unexpected exception 2147500036
14:40:07     INFO -  undefined
14:40:07     INFO -  exiting test
14:40:07     INFO -  <<<<<<<
Flags: needinfo?(prathikshaprasadsuman)
(Assignee)

Updated

a year ago
Flags: needinfo?(prathikshaprasadsuman)
(Assignee)

Updated

a year ago
Attachment #8915197 - Flags: review+ → review?(jhofmann)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8915197 - Flags: review?(jhofmann)
Attachment #8915197 - Flags: review?(jhofmann) → review+

Comment 15

a year ago
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc463de887f7
Remove the UI for indexedDB in Permissions section of PageInfo and the identity block. r=johannh
https://hg.mozilla.org/mozilla-central/rev/dc463de887f7
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox58: fix-optional → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
You need to log in before you can comment on or make changes to this bug.