Closed Bug 1289110 Opened 4 years ago Closed 4 years ago

Handle delayed delivery for presence stanzas

Categories

(Chat Core :: XMPP, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 50

People

(Reporter: abdelrahman, Assigned: abdelrahman)

Details

Attachments

(1 file, 1 obsolete file)

XEP-0203: Delayed Delivery
Attachment #8774302 - Flags: review?(aleth)
Comment on attachment 8774302 [details] [diff] [review]
v1 - delayed delivery for presence

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

::: chat/protocols/xmpp/xmpp.jsm
@@ +63,5 @@
> +  let date = getDelay(aStanza);
> +  if (date)
> +    idleSince = date.getTime();
> +  else
> +    idleSince = 0;

You can keep this case as the default, in the let clause.

@@ +87,5 @@
>  };
>  
> +// Returns a Date object for the delay value (stamp) in aStanza if it exists,
> +// otherwise returns undefined.
> +function getDelay(aStanza) {

_getDelay as this is a helper function.

@@ +95,5 @@
> +  if (delay && delay.uri == Stanza.NS.delay) {
> +    if (delay.attributes["stamp"])
> +      date = new Date(delay.attributes["stamp"]);
> +  }
> +  if ((date && isNaN(date)) || !date)

I don't understand this check.
Either date is undefined, or it's an object. If it's a Date object, when is its conversion to Number not a number? Don't you want to somehow check date is a valid date?
(In reply to aleth [:aleth] from comment #2)
> I don't understand this check.
> Either date is undefined, or it's an object. If it's a Date object, when is
> its conversion to Number not a number? Don't you want to somehow check date
> is a valid date?

I think this fixed in this patch by using |getTime| in the isNaN check and removing |!date| check.
Attachment #8774302 - Attachment is obsolete: true
Attachment #8774302 - Flags: review?(aleth)
Attachment #8774382 - Flags: review?(aleth)
Attachment #8774382 - Flags: review?(aleth) → review+
https://hg.mozilla.org/comm-central/rev/2a04b47cf89a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 50
You need to log in before you can comment on or make changes to this bug.