From bug 499654 comment 8. There's code in http://mxr.mozilla.org/mozilla-central/source/browser/base/content/sanitize.js#171 with no UI for clearing offline cache and DOM storage data: https://developer.mozilla.org/en/Offline_resources_in_Firefox#Storage_location_and_clearing_the_offline_cache https://developer.mozilla.org/en/DOM/Storage#Storage_location_and_clearing_the_data We should either kill the code or add back the UI. (BTW bug 436248 claims the code is also broken)

other issues with the code that could be re-enabled here are in dependencies. Another thing to remember is that currently Clear Recent History -> Cookies clears the DOM storage (although not always - bug 527667). The currently disabled code also clears DOM storage, so if it's enabled as is, it will be confusing. On the other hand, if it's changed, the logic in the Preferences window (Options -> Advanced -> Network -> Offline data -> Remove) should be tweaked too.

Comment on attachment 572929 [details] [diff] [review] v1 Fine by me.

(In reply to Nickolay_Ponomarev from comment #0) > We should either kill the code or add back the UI. > (BTW bug 436248 claims the code is also broken) Are there any automated tests for this functionality?

Comment on attachment 572929 [details] [diff] [review] v1 Needs also browser peer review, Ehsan, feel free to forward to anyone else. I will write test for the app cache/user data clearing. Looks like this gets closer related to bug 538588. Making the code I want to test here shared between modules and also the test seems quit reasonable now. See bug 397719 as mentioned in bug 538588 comment 13. However, bug 397719 seems to be quit old and this bug and bug 538588 are Q4 goals that need to get in ASAP. So I would rather go a followup way for sharing.

This should land after bug 538588. We also want to add the removal of offline privileges to the shared module (to fix bug 383014). The clear recent history dialog is not doing it right now.

Includes test. Deps on patch from bug 538588.

Comment on attachment 578614 [details] [diff] [review] v1.1 + test >diff --git a/browser/base/content/offlineCache.jsm b/browser/base/content/offlineCache.jsm > let OfflineCacheHelper = { > clear: function() { >+ // Remove all privileges for offline-apps Why are we including this here? For the sanitize dialog, this is handled separately by the "site specific settings" option (siteSettings), so I don't think having this option also clear permissions makes sense.

Aha, wasn't aware. I wanted to be consistent with the Advanced/Clear button, but there is now a discussion to change its behavior too.

Comment on attachment 578850 [details] [diff] [review] v1.2 + test >diff --git a/browser/base/content/sanitizeDialog.css b/browser/base/content/sanitizeDialog.css >+#itemList { >+ padding-bottom: 0.25em; >+} Why is this needed? Assuming it is needed, this should probably live in the platform-specific theme CSS file rather than this content CSS file. >diff --git a/browser/base/content/test/browser_sanitizeDialog.js b/browser/base/content/test/browser_sanitizeDialog.js >+ function () { >+ // Give www.example.com privileges to store offline data >+ var pm = Cc["@mozilla.org/permissionmanager;1"] >+ .getService(Ci.nsIPermissionManager); >+ pm.add(URI, "offline-app", Ci.nsIPermissionManager.ALLOW_ACTION); >+ pm.add(URI, "offline-app", Ci.nsIOfflineCacheUpdateService.ALLOW_NO_WARN); This should probably go in the next test, where it actually gets cleared? Or is it needed for setItem/openCacheEntry to work?

(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #10) > >+#itemList { > >+ padding-bottom: 0.25em; > >+} > > Why is this needed? Assuming it is needed, this should probably live in the > platform-specific theme CSS file rather than this content CSS file. I can see a scroll bar that disappears after clicking one of the items. This avoids that scroll bar to appear and has actually no affect on the dialog's layout. I'll try to find the proper CSS and move it there. This is AFAIR specific to Windows. > > >diff --git a/browser/base/content/test/browser_sanitizeDialog.js b/browser/base/content/test/browser_sanitizeDialog.js > > >+ function () { > > >+ // Give www.example.com privileges to store offline data > >+ var pm = Cc["@mozilla.org/permissionmanager;1"] > >+ .getService(Ci.nsIPermissionManager); > >+ pm.add(URI, "offline-app", Ci.nsIPermissionManager.ALLOW_ACTION); > >+ pm.add(URI, "offline-app", Ci.nsIOfflineCacheUpdateService.ALLOW_NO_WARN); > > This should probably go in the next test, where it actually gets cleared? Or > is it needed for setItem/openCacheEntry to work? It's obviously needed for the first test.

only thing changed: removed the bottom padding, it seems like the scroll bar is not any more appearing with the newly added item

Adding dev-doc-needed, since https://developer.mozilla.org/en/Offline_resources_in_Firefox explicitly calls out this bug as being an existing issue. (Once Firefox 11 is released, I assume we'll want to update that MDN article.)

Doc updated: https://developer.mozilla.org/en/HTML/Using_the_application_cache (the one mentioned in comment 42 has been moved to that location)