OfflineAppAllowed should not be on the IO service

RESOLVED FIXED

Status

()

Core
Networking
P1
critical
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Biesinger, Assigned: dcamp)

Tracking

({fixed1.9.1})

unspecified
fixed1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

http://hg.mozilla.org/mozilla-central/rev/3e41c4b388c3 (for bug 442806) added OfflineAppAllowed to the IO service.

That change was never reviewed.

Per discussion with bz this function should probably be somewhere in Docshell instead.

Also, the IDL method should start with a lowercase letter, and the implementation shouldn't use NS_ENSURE_SUCCESS when getting a preference value.
Flags: blocking1.9.1?
To be precise, the new implementation was never reviewed.  It makes some changes from the original implementation that seem to me to be wrong (e.g. returning error on pref not being set as opposed to just treating it as false).
(Assignee)

Comment 2

10 years ago
Created attachment 343139 [details] [diff] [review]
v1

Patch moves offlineAppAllowed to nsIOfflineCacheUpdateService, probably a better dumping ground for offline-related stuff than the docshell.

While I was at it, changed the primary offlineAppAllowed() to take a principal rather than a URI.  There's an offlineAppAllowedForURI() variant, currently used by the docshell.
Attachment #343139 - Flags: superreview?(cbiesinger)
Attachment #343139 - Flags: review?(cbiesinger)
Comment on attachment 343139 [details] [diff] [review]
v1

looks good. is it too late to fix the inconsistency in the pref name? (dashes vs underscores)
Attachment #343139 - Flags: superreview?(cbiesinger)
Attachment #343139 - Flags: superreview+
Attachment #343139 - Flags: review?(cbiesinger)
Attachment #343139 - Flags: review+
(Assignee)

Comment 4

10 years ago
(In reply to comment #3)
> (From update of attachment 343139 [details] [diff] [review])
> looks good. is it too late to fix the inconsistency in the pref name? (dashes
> vs underscores)

It's an undocumented pref that isn't included by default in any of the prefs files, so I think it should be fine to change.
(Assignee)

Comment 5

10 years ago
Landed as http://hg.mozilla.org/mozilla-central/rev/d50b1e07f56e
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Comment 6

9 years ago
This landed prior to 1.9.1 branching.
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
Priority: -- → P1

Updated

9 years ago
Depends on: 495078
You need to log in before you can comment on or make changes to this bug.