Fischer [:Fischer] Assignee
	Description • 8 years ago

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.

	Fischer [:Fischer] Assignee
	Comment 2 • 8 years ago

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.

	Fischer [:Fischer] Assignee
	Comment 3 • 8 years ago

(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

	Jared Wein [:jaws] (please needinfo? me)
	Comment 4 • 8 years ago
mozreview-review

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

	Jared Wein [:jaws] (please needinfo? me)
	Comment 5 • 8 years ago

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

	Fischer [:Fischer] Assignee
	Comment 6 • 8 years ago

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

	Jared Wein [:jaws] (please needinfo? me)
	Comment 8 • 8 years ago
mozreview-review

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.

	Fischer [:Fischer] Assignee
	Comment 9 • 8 years ago
mozreview-review-reply

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.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 11 • 8 years ago
mozreview-review

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

	Fischer [:Fischer] Assignee
	Comment 12 • 8 years ago

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

	Fischer [:Fischer] Assignee
	Comment 14 • 8 years ago

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

	Jared Wein [:jaws] (please needinfo? me)
	Comment 15 • 8 years ago
mozreview-review

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

	Jared Wein [:jaws] (please needinfo? me)
	Comment 16 • 8 years ago

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

	Fischer [:Fischer] Assignee
	Comment 17 • 8 years ago

(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

	Pulsebot
	Comment 18 • 8 years ago

Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/12e1d4360901
Hide the section of Offline Web Content and User Data in about:preferences r=jaws

	Ryan VanderMeulen [:RyanVM]
	Comment 19 • 8 years ago
bugherder

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

	Tanvir Rahman
	Comment 20 • 8 years ago

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]

	Jared Wein [:jaws] (please needinfo? me)
	Comment 21 • 8 years ago

Thanks!