1312349 - Hide the section of Offline Web Content and User Data in about:preferences

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Fischer [:Fischer]]

Remove the section of Offline Web Content and User Data under Network of Advanced of about:preferences because the management of app cache is going to integrate into the new Storage management section.

Comment 1

[Fischer [:Fischer]]

Attachment: Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

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

Comment 2

[Fischer [:Fischer]]

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

Hi Jaws,

This patch is to hide the Offline web content section in about:preferences because the new Storage management handles appcache as well.

Thanks.

Comment 3

[Fischer [:Fischer]]

(In reply to Fischer [:Fischer] from comment #2)
> Comment on attachment 8848997 [details]
> Bug 1312349 - Remove the section of Offline Web Content and User Data in
> about:preferences
> 
> Hi Jaws,
> 
> This patch is to hide the Offline web content section in about:preferences
> because the new Storage management handles appcache as well.
> 
> Thanks.
TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dcb620599eff2a833f912ea9aa3fdb209b9de425

Comment 4

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

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

https://reviewboard.mozilla.org/r/121836/#review124468

::: browser/components/preferences/in-content/advanced.js:99
(Diff revision 1)
>      setEventListener("viewSecurityDevicesButton", "command",
>                       gAdvancedPane.showSecurityDevices);
>      setEventListener("cacheSize", "change",
>                       gAdvancedPane.updateCacheSizePref);
>  
> +    if (Services.prefs.getBoolPref("browser.aboutPref.offlineGroup.enabled")) {

So what happens if this pref is disabled? Shouldn't we keep the other code path around until we are sure this will ship?

::: modules/libpref/init/all.js:5632
(Diff revision 1)
>  // To enable the DOM implementation, turn on "dom.storageManager.enabled"
>  pref("browser.storageManager.enabled", false);
>  pref("browser.storageManager.pressureNotification.minIntervalMS", 1200000);
>  pref("browser.storageManager.pressureNotification.usageThresholdGB", 5);
> +// Once the Storage Management is completed.
> +// The Offline(Appcache) Group section in about:oreferences will be hidden.

about:preferences, not about:oreferences

Comment 5

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

(In reply to Fischer [:Fischer] from comment #0)
> Remove the section of Offline Web Content and User Data under Network of
> Advanced of about:preferences because the management of app cache is going
> to integrate into the new Storage management section.

Has the work for "management of app cache and the new Storage management section" finished? I see the "Offline Web Content and User Data" section on Firefox 52 Beta. Should that section not be there on Beta or Release?

Comment 6

[Fischer [:Fischer]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> (In reply to Fischer [:Fischer] from comment #0)
> > Remove the section of Offline Web Content and User Data under Network of
> > Advanced of about:preferences because the management of app cache is going
> > to integrate into the new Storage management section.
> 
> Has the work for "management of app cache and the new Storage management
> section" finished? I see the "Offline Web Content and User Data" section on
> Firefox 52 Beta. Should that section not be there on Beta or Release?
Yes, will enable the pref to have it still display there.

Comment 7

[Fischer [:Fischer]]

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

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

Comment 8

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

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

https://reviewboard.mozilla.org/r/121836/#review125514

::: modules/libpref/init/all.js:5634
(Diff revision 2)
>  pref("browser.storageManager.pressureNotification.minIntervalMS", 1200000);
>  pref("browser.storageManager.pressureNotification.usageThresholdGB", 5);
> +// Once the Storage Management is completed.
> +// The Offline(Appcache) Group section in about:preferences will be hidden.
> +// And the task to clear appcache will be done by Storage Management.
> +pref("browser.aboutPref.offlineGroup.enabled", true);

Sorry I didn't notice this before.

Since this preference is specifically related to about:preferences we shouldn't have it in all.js. It should instead be in firefox.js.

Also, please rename this pref to:
browser.preferences.offlineGroup.enabled

And place this preference near the other browser.preferences.* prefs in firefox.js.

Comment 9

[Fischer [:Fischer]]

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

https://reviewboard.mozilla.org/r/121836/#review125514

> Sorry I didn't notice this before.
> 
> Since this preference is specifically related to about:preferences we shouldn't have it in all.js. It should instead be in firefox.js.
> 
> Also, please rename this pref to:
> browser.preferences.offlineGroup.enabled
> 
> And place this preference near the other browser.preferences.* prefs in firefox.js.

Thanks for reminding this. Moved the browser.preferences.offlineGroup.enabled pref to the firefox.js.

Comment 10

[Fischer [:Fischer]]

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

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

Comment 11

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

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

https://reviewboard.mozilla.org/r/121836/#review125986

Overall this looks good. I would have granted r+ on this, but since bug 1343682 you will need to make the changes to browser/components/preferences/in-content and browser/components/preferences/in-content-old/. Sorry :(

Comment 12

[Fischer [:Fischer]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> Comment on attachment 8848997 [details]
> Bug 1312349 - Hide the section of Offline Web Content and User Data in
> about:preferences
> 
> https://reviewboard.mozilla.org/r/121836/#review125986
> 
> Overall this looks good. I would have granted r+ on this, but since bug
> 1343682 you will need to make the changes to
> browser/components/preferences/in-content and
> browser/components/preferences/in-content-old/. Sorry :(
That's ok :) Will update based on the new changes from bug 1343682.

Comment 13

[Fischer [:Fischer]]

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

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

Comment 14

[Fischer [:Fischer]]

(In reply to Fischer [:Fischer] from comment #13)
> Comment on attachment 8848997 [details]
> Bug 1312349 - Hide the section of Offline Web Content and User Data in
> about:preferences
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/121836/diff/3-4/
Hi Jaws,
The same changes have been made to the in-content-old preferences as well.

Comment 15

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

Comment on attachment 8848997 [details]
Bug 1312349 - Hide the section of Offline Web Content and User Data in about:preferences

https://reviewboard.mozilla.org/r/121836/#review126418

Comment 16

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

This will need to be rebased now that bug 1335907 has landed.

Comment 17

[Fischer [:Fischer]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #16)
> This will need to be rebased now that bug 1335907 has landed.
Seems the bug 1335907 has not yet landed and no conflict with the central codebase so would like to continue check-in.
TRY with browser.preferences.offlineGroup.enabled as FALSE: https://treeherder.mozilla.org/#/jobs?repo=try&revision=afb17cb06014c690b4fd2addd392a1cd630b41ea
TRY with browser.preferences.offlineGroup.enabled as TRUE : https://treeherder.mozilla.org/#/jobs?repo=try&revision=a41100692dd0e3c2ec94b1b8fdf339b9ccb4248f

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12e1d4360901
Hide the section of Offline Web Content and User Data in about:preferences r=jaws

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/12e1d4360901

Comment 20

[Tanvir Rahman]

I have reproduced this issue with Nightly 52.0a1 (2016-10-24) on Windows 10, 64 bit!

The fix is now verified on Latest Nightly 55.0a1

Build ID 	20170412030252
User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170412]

Comment 21

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

Thanks!