Closed
Bug 1094632
Opened 10 years ago
Closed 10 years ago
[Woodduck][Email]No attachment icon in the right of the email which has attahments in local draft folder
Categories
(Firefox OS Graveyard :: Gaia::E-Mail, defect)
Tracking
(blocking-b2g:2.1S+, b2g-v2.0 wontfix, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.1S verified, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: jocheng, Assigned: chens)
References
Details
Attachments
(5 files)
(deleted),
text/x-github-pull-request
|
asuth
:
review+
|
Details |
(deleted),
image/png
|
Details | |
(deleted),
text/x-github-pull-request
|
asuth
:
review+
|
Details |
(deleted),
text/x-github-pull-request
|
chens
:
review+
bajaj
:
approval-gaia-v2.1+
bajaj
:
approval-gaia-v2.2+
|
Details |
(deleted),
video/3gpp
|
Details |
+++ This bug was initially created as a clone of Bug #1092945 +++
REPRODUCING PROCEDURES:
1.Enter E-Mail from idle
2.Configure a valid email account
2.Compose a email with some attachments,and save it to local draft,then go to local draft to view--KO
EXPECTED BEHAVIOUR:
should have attachment icon
Ps. I found an another issue like this, 912525, but I didn't sure that version is work for Woodduck, so I have to submit a new one, sorry.
Reporter | ||
Comment 2•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
status-b2g-v2.0:
--- → affected
status-b2g-v2.0M:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → affected
Reporter | ||
Comment 3•10 years ago
|
||
Hi Gary,
It's reported by partner. Do you think we have a temp solution for now and formal solution in future release?
Flags: needinfo?(gchen)
Comment 4•10 years ago
|
||
Thanks for spinning off the public bug. Quoting myself from that bug:
===
I can confirm the drafts implementation does not update the "hasAttachments" flag on the header in response to changes in attachments existing on the draft. I believe this was an oversight in UX specification and implementation that constitutes a low-priority bug.
===
The temp solution and the formal solution would be the same thing for this since the real fix is relatively easy.
Assignee | ||
Comment 5•10 years ago
|
||
Hi Andrew,
I think saveDraft() should be the place to update attachment flags, but I found it hard to realize the connection between message header and composer, can you shed some light on how to take care of this? thanks!
Flags: needinfo?(gchen) → needinfo?(bugmail)
Comment 6•10 years ago
|
||
Hi
any ideas? could you give me some copy from bug1092945 ?
i am not authorized to access bug #1093425.
Comment 7•10 years ago
|
||
The buggy line is basically https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/js/drafts/draft_rep.js#L50
Test coverage checking hasAttachments should be added to https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test/unit/test_compose_detach.js
Flags: needinfo?(bugmail)
Comment 8•10 years ago
|
||
(In reply to qinghao.wu from comment #6)
> any ideas? could you give me some copy from bug1092945 ?
> i am not authorized to access bug #1093425.
That bug is the same as this bug but contains partner-identifying information. There's no useful discussion or anything happening on it; we filed this bug and duped that one to this when it was clear it was a real bug.
Reporter | ||
Comment 9•10 years ago
|
||
Dear Andrew,
Do you think you can take this bug as partner is requesting the fix? Thanks!
Flags: needinfo?(bugmail)
Assignee | ||
Comment 10•10 years ago
|
||
Hi Andrew,
Thanks for the suggestion, but I didn't find 2.0/2.0m branch on gaia-email-libs-and-more repo, if I'm going to fix this on 2.0m branch, may I change the library directly in gaia repo?
Attachment #8525813 -
Flags: feedback?(bugmail)
Comment 11•10 years ago
|
||
Unfortunately, there is no 2.0/2.0m branch for gaia-email-libs-and-more. Our normal policy is to fix whatever needs to be fixed on the trunk, and then formulate an uplift manually if required. So in this case a fix would be prepared for gaia-email-libs-and-more including tests. The gaia patch would be derived from those changes via "make install-into-gaia" (as documented in gaia-email-libs-and-more's README.md) for trunk/v2.2. Then those changes would be manually applied to a v2.0 checkout.
Assignee | ||
Comment 12•10 years ago
|
||
Hi Andrew,
I found there's significant changes when "make install-into-gaia", seems the structure is totally different and I can't apply these on 2.0. If there's no 2.0 branch on gaia-email-libs-and-more repo, how should I apply these change on gaia 2.0?
Comment 13•10 years ago
|
||
Right. It is an awkward situation.
Elaborating on comment 11, the way we would normally fix this on trunk and v2.0 is:
1) Formulate a trunk fix in gaia-email-libs-and-more including a unit test.
2) Install the changes into trunk gaia using "make install-into-gaia".
3) Test the changes in trunk gaia.
4) Prepare a v2.0 branch fix by grepping for the changed code in a 2.0/2.0m branch under gaia/apps/email/js/ext and directly making the change there. (Using emacs or other tools it may be possible to apply the hunk directly.)
Comment 14•10 years ago
|
||
Comment on attachment 8525813 [details]
Pull request to 2.0m
r=asuth by inspection. I believe this is the correct fix. I should be able to "port" this to trunk and provide some unit tests over the weekend, but please do feel free to do that if you've got the spare cycles and want to learn more about the email backend tests :).
(The only tricky thing is I think there might be a cavalier unit test that abuses the mergeDraftStates function. It'll show up in the test run though.)
Attachment #8525813 -
Flags: feedback?(bugmail) → review+
Comment 15•10 years ago
|
||
Uh, which is to say, you have my blessing to land the fix on v2.0 if you can get approval. I don't want to make any claims about riskiness until we have the unit test coverage on trunk, but realistically I don't expect any risk, especially if you manually test attaching and detaching an attachment and nothing breaks or shows errors in the logs.
Reporter | ||
Updated•10 years ago
|
blocking-b2g: --- → 2.0M?
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(shchen)
Assignee | ||
Comment 16•10 years ago
|
||
Thanks Andrew :)
2.0m: https://github.com/mozilla-b2g/gaia/commit/d742e375aca6dc1bf3a36638000ad7f5338ef457
Flags: needinfo?(shchen)
Assignee | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 17•10 years ago
|
||
Let's leave this open since it's not fixed on trunk yet.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Updated•10 years ago
|
blocking-b2g: 2.0M? → 2.0M+
Reporter | ||
Comment 18•10 years ago
|
||
Hi Chens,
Can you also provide patch for 2.1 and m/c? Thanks!
Flags: needinfo?(shchen)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(shchen)
Reporter | ||
Updated•10 years ago
|
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 19•10 years ago
|
||
Hi Norry,
Please verify on Woodduck 2.0M. Thanks!
Flags: needinfo?(fan.luo)
Keywords: verifyme
Comment 20•10 years ago
|
||
Please don't mark this bug fixed until we have landed a fix on trunk.
Status: RESOLVED → REOPENED
Flags: needinfo?(bugmail)
Resolution: FIXED → ---
Comment 21•10 years ago
|
||
The bug is verified successfully on Woodduck 2.0m
Attachment: Verify.png
Reproducing rate: 0/5
Reproducing steps:
1.Enter E-Mail from idle
2.Configure a valid email account
2.Compose a email with some attachments,and save it to local draft,then go to local draft to view
Woodduck build:
Gaia-Rev 8e17537f9e6d02b7271d92134671414d1fb88386
Gecko-Rev 71be912cc4fedd423869ad9dce72880de170851f
Build-ID 20141128050313
Version 32.0
Device-Name jrdhz72_w_ff
FW-Release 4.4.2
FW-Incremental 1417122322
FW-Date Fri Nov 28 05:05:45 CST 2014
Flags: needinfo?(fan.luo)
Comment 23•10 years ago
|
||
Hi! Chens,
Will you do uplift things to m-c?
--
Keven
Flags: needinfo?(shchen)
Assignee | ||
Comment 24•10 years ago
|
||
I've already have a patch but still need to add tests, can't land on master until got review+ from module owner. Email backend test is completely different than gaia and I haven't figured it out yet.
Flags: needinfo?(shchen)
Reporter | ||
Comment 25•10 years ago
|
||
Hi Chens,
Can you take this bug since you already provide patch for 2.0M?
Thanks!
Status: NEW → ASSIGNED
Flags: needinfo?(shchen)
Assignee | ||
Comment 26•10 years ago
|
||
Took it, but I didn't have enough bandwidth to provide PR with tests for master. If anyone interested in it please feel free to take it.
Assignee: nobody → shchen
Flags: needinfo?(shchen)
Reporter | ||
Updated•10 years ago
|
Updated•10 years ago
|
blocking-b2g: 2.1S? → 2.1S+
Whiteboard: [2.1S_NotInMaster]
Assignee | ||
Comment 27•10 years ago
|
||
Hi Andrew,
This patch tries to fix hasAttachmens in draft header, and add test on it. Would you review it and give some feedback? thanks!
Attachment #8554448 -
Flags: review?(bugmail)
Assignee | ||
Comment 28•10 years ago
|
||
Hi Andrew, would you review this patch and give some feedback? thanks!
Attachment #8554452 -
Flags: review?(bugmail)
Comment 29•10 years ago
|
||
Comment on attachment 8554448 [details]
Pull request to GELAM
Thanks very much for providing the test; I know it was probably a somewhat daunting undertaking! Test looks good and passes with the fix and fails without the fix, so we're all good. I'm landing it now.
Attachment #8554448 -
Flags: review?(bugmail) → review+
Comment 30•10 years ago
|
||
landed in gaia/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/363
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/5d193a2cc170c5b4dbb0998d7608ff72cf834281
https://github.com/mozilla-b2g/gaia/pull/27675
https://github.com/mozilla-b2g/gaia/commit/4a1c90517ce8f133dc7ea96155436cb7304794ad
:chens/:josh, I'm going to leave it to you to request any appropriate uplift (or just make branch landings.)
From a risk perspective for approvers, this is extremely low risk, especially since it's already on v2.0m.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 31•10 years ago
|
||
Comment on attachment 8554452 [details]
Pull request to master
Thanks Andrew, carry over r+ and requesting for 2.1/2.2 approval
[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Pre-existed situation.
[User impact] if declined: Local draft will not have proper header indicates
there's attachment inside the draft.
[Testing completed]: Manual test and unit test.
[Risk to taking this patch] (and alternatives if risky): Quote from comment 30, this should be extremely low risk, especially since it's already on v2.0m.
[String changes made]: None
Attachment #8554452 -
Flags: review?(bugmail)
Attachment #8554452 -
Flags: review+
Attachment #8554452 -
Flags: approval-gaia-v2.2?
Attachment #8554452 -
Flags: approval-gaia-v2.1?
Comment 32•10 years ago
|
||
Comment on attachment 8554452 [details]
Pull request to master
Approving on 2.1 as its low risk enough to take it.
Attachment #8554452 -
Flags: approval-gaia-v2.2?
Attachment #8554452 -
Flags: approval-gaia-v2.2+
Attachment #8554452 -
Flags: approval-gaia-v2.1?
Attachment #8554452 -
Flags: approval-gaia-v2.1+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 33•10 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/b928887bec52e11de9af41d22ef391a9990b9776
v2.1: https://github.com/mozilla-b2g/gaia/commit/6ff41a0928e62bcfaa1b4126b4422ca51147d9c3
Updated•10 years ago
|
Comment 34•9 years ago
|
||
This problem is verified as "pass" on latest build of Flame2.1/2.1S/2.2/master and Nexus 2.2/master by the STR in comment 0.
Actual result: The attachment icon can be displayed on the right of the email in local draft folder.
See attachment: Flame_master_verify1.3gp
Rate: 0/5
Device information:
Flame 2.1 (Pass)
Build ID 20150713001203
Gaia Revision d13826b20b4a45e3f5cd4b25a30a737d8be7f1b9
Gaia Date 2015-07-02 23:36:46
Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/92049b3c4bb5
Gecko Version 34.0
Device Name flame
Firmware(Release) 4.4.2
Firmware(Incremental) eng.cltbld.20150713.035945
Firmware Date Mon Jul 13 03:59:57 EDT 2015
Bootloader L1TC000118D0
2.1S (Pass)
Build ID 20150713001205
Gaia Revision 8a614350a1f45f28e5a300bbd0233ee6d0a4738a
Gaia Date 2015-07-09 20:07:14
Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/75dd72198612
Gecko Version 34.0
Device Name scx15_sp7715ea
Firmware(Release) 4.4.2
Firmware(Incremental) 145
Firmware Date Thu Apr 16 22:58:57 CST 2015
Flame 2.2 (Pass)
Build ID 20150713002503
Gaia Revision 84d0c76370dcd3d25813b00de55194730884355b
Gaia Date 2015-07-09 13:09:14
Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8d59402ba85a
Gecko Version 37.0
Device Name flame
Firmware(Release) 4.4.2
Firmware(Incremental) eng.cltbld.20150713.041147
Firmware Date Mon Jul 13 04:11:59 EDT 2015
Bootloader L1TC000118D0
Flame master (Pass)
Build ID 20150713010204
Gaia Revision e4b63559eba364892867eb381c3002d6518e5d6a
Gaia Date 2015-07-10 14:29:23
Gecko Revision https://hg.mozilla.org/mozilla-central/rev/eab21ec484bb
Gecko Version 42.0a1
Device Name flame
Firmware(Release) 4.4.2
Firmware(Incremental) eng.cltbld.20150713.043935
Firmware Date Mon Jul 13 04:39:47 EDT 2015
Bootloader L1TC000118D0
N5 2.2 (Pass)
Build ID 20150713002503
Gaia Revision 84d0c76370dcd3d25813b00de55194730884355b
Gaia Date 2015-07-09 13:09:14
Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8d59402ba85a
Gecko Version 37.0
Device Name hammerhead
Firmware(Release) 5.1
Firmware(Incremental) eng.cltbld.20150713.040339
Firmware Date Mon Jul 13 04:04:00 EDT 2015
Bootloader HHZ12f
N5 master (Pass)
Build ID 20150713010204
Gaia Revision e4b63559eba364892867eb381c3002d6518e5d6a
Gaia Date 2015-07-10 14:29:23
Gecko Revision https://hg.mozilla.org/mozilla-central/rev/eab21ec484bb
Gecko Version 42.0a1
Device Name hammerhead
Firmware(Release) 5.1
Firmware(Incremental) eng.cltbld.20150713.043623
Firmware Date Mon Jul 13 04:36:39 EDT 2015
Bootloader HHZ12f
Updated•9 years ago
|
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in
before you can comment on or make changes to this bug.
Description
•