Closed
Bug 927784
Opened 11 years ago
Closed 11 years ago
[Messaging][Forward] Implement forward logic.
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect)
Tracking
(blocking-b2g:1.3+)
People
(Reporter: borjasalguero, Assigned: borjasalguero)
References
Details
Attachments
(1 file)
Given a message, we should be able to forward the message after tapping with a 'long press' and select this option from the list in the action menu.
Assignee | ||
Updated•11 years ago
|
Updated•11 years ago
|
Target Milestone: --- → 1.3 Sprint 3 - 10/25
Comment 1•11 years ago
|
||
Ok, didn't see this before I commented on #927783
Updated•11 years ago
|
Summary: [Messaging][Forwad] Implement forward logic. → [Messaging][Forward] Implement forward logic.
Comment 3•11 years ago
|
||
(In reply to Borja Salguero [:borjasalguero] from comment #0)
> Given a message, we should be able to forward the message after tapping with
> a 'long press' and select this option from the list in the action menu.
Borja, do you mind if I re-assign this Evelyn (from Bocoup)?
Assignee | ||
Comment 4•11 years ago
|
||
I have a patch working, only waiting to have the one which is blocking this one landed for delivering the patch, so I will follow with this work. Thanks for your support.
Updated•11 years ago
|
Target Milestone: 1.3 Sprint 3 - 10/25 → 1.3 Sprint 4 - 11/8
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #824063 -
Flags: review?(felash)
Comment 6•11 years ago
|
||
(In reply to Borja Salguero [:borjasalguero] from comment #5)
> Created attachment 824063 [details]
> Pull request
I've made some comments. We need `ThreadUI.setMessageBody` to do the handling of setting message body (seems apt, right?) because Drafts will need this same operation and it should be duplicated. The easiest approach is to just change this (in message manager):
if (message.type === 'sms') {
ThreadUI.setMessageBody(message.body);
} else {
SMIL.parse(message, function(parsedArray) {
parsedArray.forEach(function(mmsElement) {
if (!mmsElement.name) {
Compose.append(mmsElement.text);
} else {
var attachment = new Attachment(mmsElement.blob, {
name: mmsElement.name,
isDraft: true
});
Compose.append(attachment);
// Has the attachment any text associated?
if (mmsElement.text) {
Compose.append(mmsElement.text);
}
}
});
});
to this:
ThreadUI.setMessageBody(message);
And then upgrade ThreadUI.setMessageBody to:
setMessageBody: function(value) {
Compose.clear();
if (value) {
if (typeof value === 'string') {
Compose.append(value);
} else {
if (value.type === 'sms') {
Compose.append(value);
} else {
SMIL.parse(value, function(parsed) {
parsed.forEach(function(mms) {
if (!mms.name) {
Compose.append(mms.text);
} else {
var attachment = new Attachment(mms.blob, {
name: mms.name,
isDraft: true
});
Compose.append(attachment);
// Has the attachment any text associated?
if (mms.text) {
Compose.append(mms.text);
}
}
});
});
}
}
}
Compose.focus();
}
This way Drafts can use the same mechanism
Assignee | ||
Comment 7•11 years ago
|
||
Following https://github.com/mozilla-b2g/gaia/pull/13187#discussion_r7288349 I've just created a bug, https://bugzilla.mozilla.org/show_bug.cgi?id=932916, for adding this fix (due to it has implications in the current architecture, and it's needed for future features).
Comment 8•11 years ago
|
||
Two last nits on the PR, rebase and then r=me (looks like julienw was set as reviewer, I don't know if you want to wait for him to sign off on it)
Assignee | ||
Updated•11 years ago
|
Attachment #824063 -
Flags: review?(felash) → review?(waldron.rick)
Assignee | ||
Comment 9•11 years ago
|
||
Hi Rick. I've just updated the code with the final nits and now it's rebased & squashed. Thanks for your quick review!
Assignee | ||
Updated•11 years ago
|
Attachment #824063 -
Flags: review?(felash)
Comment 10•11 years ago
|
||
Comment on attachment 824063 [details]
Pull request
added comments on github
nothing big, should be easy to fix
Comment 11•11 years ago
|
||
Comment on attachment 824063 [details]
Pull request
r=me
thanks
let's wait for a green travis ?
Attachment #824063 -
Flags: review?(felash) → review+
Assignee | ||
Comment 12•11 years ago
|
||
Rick, Do you have any other comment? If you think that this is r+, could you update the flag for ensuring that all comments are addressed? Thanks!
Flags: needinfo?(waldron.rick)
Comment 13•11 years ago
|
||
Comment on attachment 824063 [details]
Pull request
LGTM
Attachment #824063 -
Flags: review?(waldron.rick) → review+
Comment 14•11 years ago
|
||
master: b0cc3bf9fefe292f2dcbdd4ceba88c97ea700369
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Flags: needinfo?(waldron.rick)
You need to log in
before you can comment on or make changes to this bug.
Description
•