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

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
5 years ago
5 years ago

People

(Reporter: julienw, Assigned: fcampo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
(Assignee)

Comment 1

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

WDYT Julien?
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: needinfo?(felash)
Resolution: --- → WORKSFORME
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 :)
Status: RESOLVED → REOPENED
Flags: needinfo?(felash)
Resolution: WORKSFORME → ---
Summary: update the cursor code landed in Bug 859662 → replace "if (this.result)" by "if (!this.done)" in the MessageManager's cursor code
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
(Assignee)

Comment 4

5 years ago
Created attachment 761988 [details] [diff] [review]
Patch
Attachment #761988 - Flags: review?(felash)
Comment on attachment 761988 [details] [diff] [review]
Patch

r=me, cheers !
Attachment #761988 - Flags: review?(felash) → review+
(Assignee)

Comment 6

5 years ago
merged on master: cd662bb810244f14171a754def91f1f49a2ec44f
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

5 years ago
:( backing out for causing a regression, sorry 381746f317c831cf44db978890847772462ba24e
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
What's the regression ?

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

Comment 9

5 years ago
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
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.
Assignee: nobody → fernando.campo
Works fine with the current code, so let's not try to touch it.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.