1334411 - Obsolete UI for IndexedDB in Page Info/Permissions

Status: RESOLVED FIXED

(Firefox :: Page Info Window, defect, P3)

Description

[YUKI "Piro" Hiroshi]

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.)

Comment 1

[:prathiksha]

Attachment: Bug 1334411 - Remove the UI for indexedDB in Permissions section of PageInfo and the identity block.

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

Comment 2

[Johann Hofmann [:johannh]]

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

Comment 3

[Johann Hofmann [:johannh]]

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 4

[:prathiksha]

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

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

Comment 5

[Johann Hofmann [:johannh]]

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.

Comment 7

[:prathiksha]

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

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

Comment 8

[Johann Hofmann [:johannh]]

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.

Comment 9

[:prathiksha]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186434/diff/3-4/

Comment 10

[Johann Hofmann [:johannh]]

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!

Comment 11

[:prathiksha]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186434/diff/4-5/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f223f11a08f
Remove the UI for indexedDB in Permissions section of PageInfo and the identity block. r=johannh

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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 -  <<<<<<<

Comment 14

[:prathiksha]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186434/diff/5-6/

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc463de887f7
Remove the UI for indexedDB in Permissions section of PageInfo and the identity block. r=johannh

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/dc463de887f7