Closed Bug 995605 Opened 10 years ago Closed 10 years ago

[Sora][Message][MMS]Can't add the text in content when forward a media MMS.


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



(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

1.4 S6 (25apr)
blocking-b2g 1.3+
Tracking Status
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed


(Reporter: sync-1, Assigned: julienw)




(Whiteboard: [cert][ETA:20140428][p=1])


(1 file)

46 bytes, text/x-github-pull-request
: review+
Details | Review
Mozilla build ID:20140323004002
 ->MS send/received a MMS,the MMS content only have media files;
 ->Long tap the MMS,select forward the MMS;
 ->Tap the content pane to edit,the keyboard can't display,it means can't add the text in content.(ko)
 ->Back to edit other message interface,can't use the virtual keyboard to input text in content.(ko)
  For FT PR, Please list reference mobile's behavior:
blocking-b2g: --- → 1.3?
Keywords: regression
Forwarding is new to 1.3 so it can't be a regression from a previous version.

That said, this is in my opinion serious because after this bug happen we can't focus the composer anymore, even in another thread or when starting a new message !
Can we reproduce this on the Moz side?
Keywords: regressionqawanted
Not able to reproduce this bug in latest 1.3 version
hamachi v1.3
Build Identifier-20140412061954
Backlog then if it's not reproducible.
blocking-b2g: 1.3? → backlog
Keywords: qawanted
Whiteboard: [closeme 4/19/2014]
FYI I could reproduce very easily but I have a slightly older build, I'll try again with a newer build.
Maria, I can reproduce very easily... Could you recheck on your side?

I'd like this to be at least a 1.4 blocker because it's really awkward.
blocking-b2g: backlog → 1.4?
Flags: needinfo?(oteo)
You are right Julien, sorry for the inconvenience, it seems I didn't follow the STR correctly...

I am reproducing the bug in latest v1.3 version
hamachi v1.3

and agree that it's an awful error and IMHO we should fix it in v1.3

So nominating to v1.3 because once the error happens it breaks totally the SMS application as you can not use the keyboard in the composer anymore.

I had to kill the SMS application and launch it again to make the keyboard work and be able to send a SMS.
blocking-b2g: 1.4? → 1.3?
Flags: needinfo?(oteo)
I think it would be easy to fix: just ensure there is a text placeholder after forwarding (I think Compose.fromMessage is the place).
triage: blocking 1.4+ but please ask for approval to 1.3 when the patch is ready. thanks
blocking-b2g: 1.3? → 1.4+
(In reply to Joe Cheng [:jcheng] from comment #9)
> triage: blocking 1.4+ but please ask for approval to 1.3 when the patch is
> ready. thanks

We don't grant approvals for 1.3 anymore. They have be cert blockers to be taken at this point.

Vance - Can you find out if this is a cert blocker for TCL?
Flags: needinfo?(vchen)
Whiteboard: [closeme 4/19/2014]
Yes this is a cert blocker for TCL
Flags: needinfo?(vchen)
blocking-b2g: 1.4+ → 1.3+
Whiteboard: [cert]
Assignee: nobody → felash
Whiteboard: [cert] → [cert][ETA:20140425]
Whiteboard: [cert][ETA:20140425] → [cert][ETA:20140425][p=1]
Whiteboard: [cert][ETA:20140425][p=1] → [cert][ETA:20140428][p=1]
Target Milestone: --- → 2.0 S1 (9may)
Attached file github PR
The issue is that in compose.js, "pointer-events: none" is added after it's
    removed, in this case.
    This patch ensures that SMIL.parse always calls the callback in an asynchronous way,
    so that the caller does not have any surprise.

 apps/sms/js/desktop-only/mobilemessage.js | 18 ++++++++++++++++++
 apps/sms/js/smil.js                       |  2 +-
 apps/sms/test/unit/smil_test.js           | 25 +++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)
Attachment #8412554 - Flags: review?(schung)
This is a good lesson: I always say it's a good practice to ensure that calling a callback should always be done either in an asynchronous or synchronous way, but here is a very concrete bug coming from the face we didn't respect the good practice.

Note that Promises are always asynchronous by design, so it's great to use them :) Maybe we should eventually use a Promise here too, but since it's a 1.3+ bug, I'd rather be conservative and safe.
Comment on attachment 8412554 [details] [review]
github PR

Only some nits for the comment part, thanks for the quick fix!
Attachment #8412554 - Flags: review?(schung) → review+
Thanks for the quick review !

master: c6f4657a4ec2d9fc4fff7b7b8f96f91ef3bd13c2
Closed: 10 years ago
Resolution: --- → FIXED

Please request approval-gaia-v1.3 on this patch when it is ready for 1.3 uplift.
Target Milestone: 2.0 S1 (9may) → 1.4 S6 (25apr)
Flags: needinfo?(felash)
Comment on attachment 8412554 [details] [review]
github PR

NOTE: Please see to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 976999
[User impact] if declined: after forwarding an image-only MMS, the user can't interact with the composer, even in other panels, unless he forwards another MMS or restart the application.
[Testing completed]: yes
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none
Attachment #8412554 - Flags: approval-gaia-v1.3?
Flags: needinfo?(felash)
Attachment #8412554 - Flags: approval-gaia-v1.3? → approval-gaia-v1.3?(fabrice)
Attachment #8412554 - Flags: approval-gaia-v1.3?(fabrice) → approval-gaia-v1.3+
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.