1202219 - [email/UI] Editing a draft from the outbox fails with "this.model is undefined" due to missing arrow-function/bind.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Shine(Leave from Mozilla)]

Attachment: logcat_AriesKK2.5_0334.txt

[1.Description]:
[Aries KK v2.5][Flame KK v2.5][Email]Edit an email and save it to outbox, the subject and content of email will lose, tap the Send icon/Save to Local Draft button, there're no response; Turn on wifi and  tap the Send icon/Save to Local Draft button again, there're no response too.
See attachment: AriesKK2.5_video2.3gp & logcat_AriesKK2.5_0334.txt
Found time: 03:34

[2.Testing Steps]: 
Precondition: The network is unavailable.
1. Launch email app 
2. Edit an email and tap Send icon, email will save to outbox.
3. Go to outbox 
4. Select the mail to enter detail screen
5. Tap on the send icon 
6. Tap Back icon and tap 'Save to Local Draft' button
7. Turn on wifi from Notification bar and repeat step 5~6

[3.Expected Result]: 
Step 4. The subject and content of email should not be lost.
Step 5. The mail could be resent and a network prompt message will pop up.
Step 6. The mail could be saved to Local Draft successfully.
Step 7. The mail could be resent/saved to Local Draft successfully.

[4.Actual Result]: 
Step 4. The subject and content of email were lost.
Step 5. There's no response, the mail can't be resent.
Step 6. There's no response, the mail can't be saved to Local Draft.
Step 7. The mail can‘t be resent/saved to Local Draft too.

[5.Reproduction build]: 
Device: Aries KK v2.5 (Affected)
Build ID               20150905112803
Gaia Revision          03be7d8918bed58c92a40bba211bbbc97a0e516a
Gaia Date              2015-09-04 11:56:30
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/139446daedab3d00b9f0faba25e1e82191c8c8d1
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150905.105729
Firmware Date          Sat Sep  5 10:57:37 UTC 2015
Bootloader             s1

Device: Flame KK v2.5 (Affected)
Build ID               20150905150219
Gaia Revision          03be7d8918bed58c92a40bba211bbbc97a0e516a
Gaia Date              2015-09-04 11:56:30
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/139446daedab3d00b9f0faba25e1e82191c8c8d1
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150905.183746
Firmware Date          Sat Sep  5 18:38:03 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Flame KK v2.2 (Unaffected)
Build ID               20150905032501
Gaia Revision          335cd8e79c20f8d8e93a6efc9b97cc0ec17b5a46
Gaia Date              2015-08-14 19:06:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/d3cb4f28c735
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150905.065255
Firmware Date          Sat Sep  5 06:53:06 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
11535

Comment 1

[Shine(Leave from Mozilla)]

Attachment: AriesKK2.5_video2.3gp

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] asutherland:email-draft-compose-this-glitch > mozilla-b2g:master

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8657612 [details] [review]
[gaia] asutherland:email-draft-compose-this-glitch > mozilla-b2g:master

I was worried this was a complicated drafts, problem, but it turns out that in the refactor (presumably), we ended up with a non-bind()-wrapped function in between the arrow function use that helped propagate "this.model".  Since an undefined "this" was latched, sadness.

Note that this is just a candidate by inspection, I haven't tested on device and there's no easy way I can find to make b2g-desktop actually be offline.  Feel free to take over the patch or r+ or what not.  We probably do want to do an additional verification pass on the bug 1196077 refactoring to make sure no other instances slipped through.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Also, fantastic find on this, Shine!  Thank you very much!  As usual, your providing both a log and a video was invaluable and made identifying the problem quite easy.

Comment 5

[James Burke [:jrburke]]

Comment on attachment 8657612 [details] [review]
[gaia] asutherland:email-draft-compose-this-glitch > mozilla-b2g:master

This did indeed fix the issue when I test on device, and I did another pass at all the this.model uses. I did not find any more, but then again, I missed this one on the first pass.

For this particular case, maybe we can construct an eslint rule for it. Not sure if rules have enough context to do this kind of restriction, brief scan of eslint rules did not seem to have one, but something I might keep in the back of my thoughts.

For this issue though, this pull request is enough. I will take care of merging.

Comment 6

[James Burke [:jrburke]]

Merged in master:
https://github.com/mozilla-b2g/gaia/commit/f58120447bc37eafae03ba83cf0ddc129207eca1

from pull request:
https://github.com/mozilla-b2g/gaia/pull/31711

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

(In reply to James Burke [:jrburke] from comment #5)
> For this particular case, maybe we can construct an eslint rule for it. Not
> sure if rules have enough context to do this kind of restriction, brief scan
> of eslint rules did not seem to have one, but something I might keep in the
> back of my thoughts.

This existing eslint rule seems to be what we want:
http://eslint.org/docs/rules/no-invalid-this

Specifically, with a test snippet like this that's part of a prototype dict:
  foo: function foo() {
    this.blah(function bar() { // arrow goes away if I make this a fat-arrow function.
      return () => {
        return this.baz; // Unexpected `this` here from "no-invalid-this" rule.
      };
    }); // error goes away if I put bind here.
  }

I've added the eslint rule to the gelam convoy branch. rs=asuth on modifying any relevant gaia lint rules for the email app to include this as a warning or error, but feel free to ask for review if you desire.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

er, and I of course meant "error goes away", not "arrow goes away".

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

One additional wrinkle with the eslint rule is that if co.wrap() is used to decorate a generator function, the rule freaks out.  So if it was "foo: co.wrap(function*() { this.blah(); }", then the rule goes nuts.

I'm not quite sure what to do about this.  Options would seem to be:
- Remove co.wrap() from all my class-idiom definitions...
  - Make it the caller's problem to coerce to a Promise via attaching a co.wrap driver as needed.  In convoy this is fairly easy in general since this is mainly just used via the task infrastructure that literally only has 2 call-sites.  The places where I was getting burnt by yield* before may be a hassle though.
  - Pass the prototype through a transformation/decorating step like evt.mix where generator functions get detected and wrapped with co.wrap.
- Fork the rule.
- Add /** @this */ to my co.wrap boilerplate usage so it becomes: "foo: /** @this */ co.wrap(...)" which I think makes things work.

Comment 10

[Jessica]

Attachment: verified_Aries KK v2.5.3gp

According to the STR of Comment 0, this bug has been verified as pass on latest Flame KK v2.5 and Aries KK v2.5.

Actual results: Step 4. The subject and content of email should not be lost.
Step 5. The mail could be resent and a network prompt message will pop up.
Step 6. The mail could be saved to Local Draft successfully.
Step 7. The mail could be resent/saved to Local Draft successfully.

See attachments: verified_Aries KK v2.5.3gp
Reproduce rate: 0/6


Device: Flame KK v2.5 (Pass)
Build ID               20150907150218
Gaia Revision          891798f1e345bc2b69e71de42bd524a90b1745c4
Gaia Date              2015-09-07 02:49:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/5fe9ed3edd6811a662d40d05e37b0d66e9520d82
Gecko Version          43.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150907.182946
Firmware Date          Mon Sep  7 18:29:58 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK v2.5 (Pass)
Build ID               20150907110522
Gaia Revision          891798f1e345bc2b69e71de42bd524a90b1745c4
Gaia Date              2015-09-07 02:49:56
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/5fe9ed3edd6811a662d40d05e37b0d66e9520d82
Gecko Version          43.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150907.102632
Firmware Date          Mon Sep  7 10:26:39 UTC 2015
Bootloader             s1