863167 - replace "if (this.result)" by "if (!this.done)" in the MessageManager's cursor code

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::SMS, defect)

Description

[Julien Wajsberg [:julienw]]

In Bug 859662 we landed a different patch on v1-train than what landed on master.

Therefore I'd like that we update master to have the same code than v1-train at this place, so that we don't have conflicts later.

Comment 1

[Fernando Campo (:fcampo)]

I think that this is outdated, I don't see many differences with the patches and current code.

WDYT Julien?

Comment 2

[Julien Wajsberg [:julienw]]

nope, it's still there :)

It's only how we check that we're not at the end of the iteration. On master we do "if (this.result)" whereas on v1-train we do "if (!this.done)" which is better.

So that's really simple to do :)

Comment 3

[Julien Wajsberg [:julienw]]

The conflict resolution for Bug 868679 [1] put the master code on v1-train, whereas the code in v1-train was "more right". So bugmorphing this bug.

[1] https://github.com/mozilla-b2g/gaia/commit/bda16a078d6ba272f5f975df6a21c9200fce7051

Comment 4

[Fernando Campo (:fcampo)]

Attachment: Patch

Comment 5

[Julien Wajsberg [:julienw]]

Comment on attachment 761988 [details] [diff] [review]
Patch

r=me, cheers !

Comment 6

[Fernando Campo (:fcampo)]

merged on master: cd662bb810244f14171a754def91f1f49a2ec44f

Comment 7

[Fernando Campo (:fcampo)]

:( backing out for causing a regression, sorry 381746f317c831cf44db978890847772462ba24e

Comment 8

[Julien Wajsberg [:julienw]]

What's the regression ?

I tested this (lightly) and it seemed to work...

Comment 9

[Fernando Campo (:fcampo)]

this.result.id appears as null, and messages screen doesn't load, which honestly, I don't understand, but I preferred to back out till I make more testing on device later today

Comment 10

[Julien Wajsberg [:julienw]]

It would mean that we could have this.result as null even if this.done is false ?

Then I'd say we need to do both checks :
- this.done to know if we're at the end of the cursor; if result can be null in the middle of the cursor, we don't want to stop here
- this.result so that we don't choke if it's null

If this.done is never true, then it's a bug in the gecko API.

Comment 11

[Julien Wajsberg [:julienw]]

Works fine with the current code, so let's not try to touch it.