Closed Bug 1216683 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: DOM: Push Notifications, defect)

Product:
Core
Component:
DOM: Push Notifications
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla44
Tracking Flags:
Tracking Status
firefox44 --- fixed

People

(Reporter: dragana, Assigned: dragana)

Details

Attachments

(1 file, 1 obsolete file)

bug_1216683.patch
2.49 KB, patch
lina
: review+
Details | Diff | Splinter Review 
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: 8 years ago

          status-firefox44:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44

          You need to log in
          before you can comment on or make changes to this bug.