Closed Bug 1216683 Opened 4 years ago Closed 4 years ago

PushServiceWebSocket: unregister should always return true even if we are offline

Categories

(Core :: DOM: Push Notifications, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: dragana, Assigned: dragana)

Details

Attachments

(1 file, 1 obsolete file)

I have spotted that if we are offline ( also if pref connection.enable is false )
unregister will return false. 
For the Http2 version that is by the spec but for WebSocket version we should return true always (even if we do not sen unregister request to the push server).
Attached patch bug_1216683.patch (obsolete) — Splinter Review
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Attachment #8679440 - Flags: review?(kcambridge)
Comment on attachment 8679440 [details] [diff] [review]
bug_1216683.patch

Review of attachment 8679440 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/push/PushService.jsm
@@ +924,5 @@
>    _sendRequest: function(action, aRecord) {
>      if (this._state == PUSH_SERVICE_CONNECTION_DISABLE) {
>        return Promise.reject({state: 0, error: "Service not active"});
>      } else if (this._state == PUSH_SERVICE_ACTIVE_OFFLINE) {
> +      if (this._serviceType == "WebSocket" && action == "unregister") {

Should this be `this._service.serviceType() == "WebSocket"`?
Attachment #8679440 - Flags: review?(kcambridge) → feedback+
:)
Attachment #8679440 - Attachment is obsolete: true
Attachment #8679515 - Flags: review?(kcambridge)
Comment on attachment 8679515 [details] [diff] [review]
bug_1216683.patch

Review of attachment 8679515 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome, thanks!
Attachment #8679515 - Flags: review?(kcambridge) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b6cf4c726f70
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.