933993 - Add (more) UI for appcache alert/permissions

Status: RESOLVED WONTFIX

(Firefox :: Settings UI, defect)

Description

[philsummer]

User Agent: Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131028225529

Steps to reproduce:

Since firefox 26 "There is no longer a prompt when websites use appcache" ( http://www.mozilla.org/en-US/firefox/26.0/releasenotes/ ). 

But "Menu bar > Edit> Preferences > Advanced > Network > Alerts when a website request storage for offline use" is still checked but does not warn the user. Moreover in "about:permissions" blocking one website is not working anymore.

 * open http://www.whatwg.org/demos/offline/clock/live-demo/clock.html
 * notice there's no alert while "Alerts when a website request storage for offline use" is still checked in preferences.
 * close the clock tab 
 * empty offline data

 * go to http://www.whatwg.org/specs/web-apps/current-work/multipage/offline.html#offline
 * open another tab to "about:permissions" and forbid whatwg.org to save offline data
 * open http://www.whatwg.org/demos/offline/clock/live-demo/clock.html
 * switch to offline mode
 * reopen http://www.whatwg.org/demos/offline/clock/live-demo/clock.html
 * notice that the data were saved in appcache while specifically forbidden in "about:permissions"


Actual results:

appcache now ignore the user settings for both "alert" and "about:permissions"



Expected results:

The alert setting should be disabled by default, but still be working if changed by user (See 887102; 648064).

The "about:permission" settings should be respected.

Comment 1

[Honza Bambas (:mayhemer)]

Regarding your concern on the preferences dialog:

The preference dialog is wrong.  We have two preferences regarding the appcache prompt:

- browser.offline-apps.notify / bool / default at true
- offline-apps.allow_by_default / bool / default at true (after bug 892488)

The UI is bound to browser.offline-apps.notify pref.  This has not been changed by bug 892488.

We never had a UI for offline-apps.allow_by_default.  We have to add it.



Regarding you about:permissions concern:

Those are completely wrong either.  The "Maintain Offline Storage" text is completely misleading since it is bound to "indexDB" permission and default setting only and has nothing to do with the appcache prompting.



Then, I think nsContentUtils::MaybeAllowOfflineAppByDefault has a bug.  When you explicitly disable the "offline-app" permission for a host (however, you have no chance to do it from any UI :)), then the method should not override it.  We must add a check for the permission state whether is in state "UNKNOWN".


Thanks for the report.


WORKAROUND: If you want to re-enable the prompt, just switch the pref offline-apps.allow_by_default to false in about:config.

Comment 2

[Honza Bambas (:mayhemer)]

Dao, are you willing to fix this bug?

According my job here, I will separate the nsContentUtils::MaybeAllowOfflineAppByDefault bit to a new bug.  I presume you will fix the about:permission UI by: 1) correctly naming the current "offline"/indexDB option and 2) add new for the offline permission.  Then we need to respect denial of the offline permission when offline-by-default is on.  Currently we don't.

Comment 3

[Dão Gottwald [:dao]]

You made this block bug 885366, but that UI is different from about:permissions. Are they both affected?

Comment 4

[Honza Bambas (:mayhemer)]

I've found that bug via search for 'Maintain Offline Storage' text that first appears (and is used by about:permissions) at https://bugzilla.mozilla.org/attachment.cgi?id=769923&action=diff#a/browser/locales/en-US/chrome/browser/sitePermissions.properties_sec2

Comment 5

[Dão Gottwald [:dao]]

(In reply to Honza Bambas (:mayhemer) from comment #4)
> I've found that bug via search for 'Maintain Offline Storage' text that
> first appears (and is used by about:permissions) at
> https://bugzilla.mozilla.org/attachment.cgi?id=769923&action=diff#a/browser/
> locales/en-US/chrome/browser/sitePermissions.properties_sec2

about:permissions uses browser/locales/en-US/chrome/browser/preferences/aboutPermissions.dtd.

Comment 6

[Honza Bambas (:mayhemer)]

(In reply to Dão Gottwald [:dao] from comment #5)
> about:permissions uses
> browser/locales/en-US/chrome/browser/preferences/aboutPermissions.dtd.

Doesn't matter, still, the label name is 'indexedDB.label'.  So I'm on the right address.

There is however this comment:

LOCALIZATION NOTE (indexedDB.label): This is describing indexedDB storage using the same language used for the permIndexedDB string in browser/pageInfo.dtd 

Bug I don't see permIndexedDB in that file...  Added in bug 594583, removed in bug 889835.

Comment 7

[Dão Gottwald [:dao]]

(In reply to Honza Bambas (:mayhemer) from comment #6)
> (In reply to Dão Gottwald [:dao] from comment #5)
> > about:permissions uses
> > browser/locales/en-US/chrome/browser/preferences/aboutPermissions.dtd.
> 
> Doesn't matter, still, the label name is 'indexedDB.label'.

I'm confused. This bug is about about:permissions (right?) and you made it block bug 885366 because you thought about:permissions used browser/locales/en-US/chrome/browser/sitePermissions.properties -- but it doesn't and never did.

(In reply to Honza Bambas (:mayhemer) from comment #6)
> There is however this comment:
> 
> LOCALIZATION NOTE (indexedDB.label): This is describing indexedDB storage
> using the same language used for the permIndexedDB string in
> browser/pageInfo.dtd 
> 
> Bug I don't see permIndexedDB in that file...  Added in bug 594583, removed
> in bug 889835.

Right, the page info dialog is older than about:permissions, so there was a note added to the latter such that localizers would use the same wording.

Comment 8

[Honza Bambas (:mayhemer)]

OK, let's unblock the bug.  However, the bad label had been introduced in some bug, so we should at least make clear which one it is and block on it.  But that is definitely not needed to fix this bug.

Comment 9

[:Gijs (he/him)]

Per comment #1 this basically requires additional UI, which we won't create because appcache is on the way out ( bug 1237782 ).