[Messages] Read receipts feature Gaia implementation

RESOLVED FIXED

Status

Firefox OS
Gaia::SMS
P1
blocker
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: steveck, Assigned: steveck)

Tracking

({feature})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ucid:Comms27, 1.3:p2, ft:comms])

Attachments

(1 attachment)

46 bytes, text/x-github-pull-request
julienw
: review+
arthurcc
: review+
Details | Review | Splinter Review
(Assignee)

Description

5 years ago
To match the criteria in User Story(bug 919974), the necessary implementation in gaia will be:

- Having a new option for read report in settings app/mesage settings panel.
- Handling messageRead event callback and update the thread ui page.
- Handling messageRead attribute in message object and display the message read icon.
blocking a committed 1.3 user story. 1.3+
blocking-b2g: 1.3? → 1.3+
Target Milestone: --- → 1.3 Sprint 5 - 11/22
blocking-b2g: 1.3+ → ---
Whiteboard: [ucid:Comms27, 1.3:p1] → [ucid:Comms27, 1.3:p2]
Target Milestone: 1.3 Sprint 5 - 11/22 → ---
Whiteboard: [ucid:Comms27, 1.3:p2] → [ucid:Comms27, 1.3:p2, ft:comms]
(Assignee)

Comment 2

5 years ago
Created attachment 8371354 [details] [review]
Link to github

Hi Julien, this patch enable the read report feature which is postponed from v1.3 and gecko side is already ready in master(gecko has one bug about the read report I found today, but it won't break the functionality and I will set it as blocker later). New thread ui view layout for delivered/read status is still unclear for me, so I keep the original tickle for both status and I'll update with thread ui view refresh. If you want to test with other message app, please make sure the target app support read report feature.
Attachment #8371354 - Flags: review?(felash)
Depends on: 968750
Comment on attachment 8371354 [details] [review]
Link to github

Sorry for being late, 1.3 sucked up all my time :(

Mostly nits, except the "live update the information report" part, that's why I'm not giving r+ yet.

Also, I couldn't check it works locally. I correctly get the "readsuccess" event but I miss the various properties.

Also, you'll need a review from a settings peer.

But this is otherwise a quite solid patch!
Attachment #8371354 - Flags: review?(felash)
(Assignee)

Comment 4

5 years ago
Hi Julien,
May I know what kind of properties missed in you local build? Only read receipt related (readStatus/readTimestamp) or even delivery properties are also missed in deliveryInfo?

Hi Gene/Bevis,
Julien faced some problem in read receipt. In comment 3, he said he can get readsuccess event but there seems missing some properties related to read receipt. Is that possible?
Flags: needinfo?(gene.lian)
Flags: needinfo?(btseng)
The delivery properties were fine.

To find this, I simply debugged on the phone and put a breakpoint in "createReportDiv" to inspect the "reports" variable.
Not I'm not sure I had the readsuccess event, I thought I had it because I saw the green "tick" but maybe it was the delivery report instead. I'll chcek again today.

Anyway, do I need a special gecko patch to enable this, or is it supposed to be already in central?
(Assignee)

Updated

5 years ago
Blocks: 971658
(Assignee)

Comment 7

5 years ago
Comment on attachment 8371354 [details] [review]
Link to github

Hmm...the only thing need to do here is open the read receipt option in settings page and it should work on current master without any other trick. Please make sure both device flash to master with this patch, or make sure your android device will return read receipt.
Attachment #8371354 - Flags: review?(felash)
Attachment #8371354 - Flags: review?(arthur.chen)
(Assignee)

Updated

5 years ago
Flags: needinfo?(btseng)
Clear myself based on comment #6.
Flags: needinfo?(gene.lian)
Comment on attachment 8371354 [details] [review]
Link to github

Please check the github comment, thanks.
Attachment #8371354 - Flags: review?(arthur.chen)
(In reply to Steve Chung [:steveck] from comment #7)
> Comment on attachment 8371354 [details] [review]
> Link to github
> 
> Hmm...the only thing need to do here is open the read receipt option in
> settings page and it should work on current master without any other trick.
> Please make sure both device flash to master with this patch, or make sure
> your android device will return read receipt.

I was sending a SMS to myself ;)

I'm building a new gecko and we'll see.
(Assignee)

Updated

5 years ago
Attachment #8371354 - Flags: review?(arthur.chen)
(Assignee)

Comment 11

5 years ago
Messageing settings updated. Although I think we don't need to hide these message options while airplane mode, it would be safer to discuss with Jose first. We will create another bug for this.
Comment on attachment 8371354 [details] [review]
Link to github

r=me for the settings part. Thanks.
Attachment #8371354 - Flags: review?(arthur.chen) → review+
Comment on attachment 8371354 [details] [review]
Link to github

r=me

I'm so dumb, because I tried with SMS and not with MMS...

thanks Steve !
Attachment #8371354 - Flags: review?(felash) → review+
Mmm you have unit tests issues in settings (the calendar one seems unrelated), so please check it before merging.
(Assignee)

Comment 15

5 years ago
(In reply to Julien Wajsberg [:julienw] from comment #14)
> Mmm you have unit tests issues in settings (the calendar one seems
> unrelated), so please check it before merging.

Thanks for the reminder, I'll commit a test-fixing pitch for Arthur soon!
(Assignee)

Comment 16

5 years ago
Comment on attachment 8371354 [details] [review]
Link to github

Hi Arthur, I added some fixing in messaging_test. Sorry for bothering you again...
Attachment #8371354 - Flags: review+ → review?(arthur.chen)
Comment on attachment 8371354 [details] [review]
Link to github

r=me. Thanks!
Attachment #8371354 - Flags: review?(arthur.chen) → review+
(Assignee)

Comment 18

5 years ago
Thanks for all the review!
Landed in master: 14a578d181e76d9fd763d16ef2c23c20d00a9b16
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 19

5 years ago
I find

Request read report for each message sent

hard to understand in English. I know that this showed up in wireframes a lot, but is this the best English string we can get?
Axel, can you file a new bug? I'm sure we can do better, we can ask Tyler Altes about this.
(In reply to Axel Hecht [:Pike] from comment #19)
> I find
> 
> Request read report for each message sent
> 
> hard to understand in English. I know that this showed up in wireframes a
> lot, but is this the best English string we can get?

I think adding "a" makes it more clear:

Request a read report for each message sent
Tyler, could you please proofread this text (ref: comment 19 and comment 21) ?

Thanks !
Flags: needinfo?(tyler.altes)

Comment 23

5 years ago
Hi, I would actually change most of the language around this function. 

First, a question to make sure I understand: When we say "request notification of delivery", do they actually receive a notification or just a green checkmark within the message details when they open the message/status?

For wording, instead of "Delivery" and "Read", I would talk about "Sent" and "Opened". And then, the concept of a "report" and "requesting a report" sounds confusing and technical. Here's how I would put the texts: 

For the Settings screen - https://bug919974.bugzilla.mozilla.org/attachment.cgi?id=8336259:

"Delivery Report" -> "Notify when sent"
"You'll see a check when a message is sent successfully."

"Read Report" -> "Notify when opened"
"You'll see a check when a message you sent is opened."


And on this screen - https://bug919974.bugzilla.mozilla.org/attachment.cgi?id=8336260:

"Message Report" -> "Message Details"

"Delivery: 05th october 2013 at 11:45" -> "Sent: 5 October 2013 at 11:45"

"Read: Report requested" -> "Opened: pending"

Hope all this is clear. Let me know if you need more feedback.
Flags: needinfo?(tyler.altes)
I would make a couple of small tweaks:

"Delivery Report" -> "Notify when sent"
"You'll see a check mark when your message is sent successfully."

"Read Report" -> "Notify when opened"
"You'll see a check mark when the message you sent is opened."

My only concern is how well "check mark" is going to localize.

Comment 25

5 years ago
a) what do we do with this bug? Back out and re-land with the right strings?

b) I think we can create a localization note for "You'll see a check mark..." explaining that there's a green check mark icon appearing next to message. It should be possible to find a phrase for that, independent of the word check mark.

Comment 26

5 years ago
I agree with Matej that it's a bit clearer saying "check mark", but I wanted to keep it within the same character count as the previous message. If the phrase fits with "mark", even better.

And Axel's right. If the translators know what we're referring to, they should be able to find a way to explain it, even if they don't translate a direct name for check mark. Another option would be to use the symbol instead of the word: "You'll see a √ when your message is sent successfully." Not sure how practical it is on a technical/font level...
(In reply to tyler.altes from comment #23)
> Hi, I would actually change most of the language around this function. 
> 
> First, a question to make sure I understand: When we say "request
> notification of delivery", do they actually receive a notification or just a
> green checkmark within the message details when they open the message/status?

The green checkmark, and it's also displayed when displaying a message details.


> For wording, instead of "Delivery" and "Read", I would talk about "Sent" and
> "Opened". And then, the concept of a "report" and "requesting a report"
> sounds confusing and technical. Here's how I would put the texts: 
> 
> For the Settings screen -
> https://bug919974.bugzilla.mozilla.org/attachment.cgi?id=8336259:
> 
> "Delivery Report" -> "Notify when sent"
> "You'll see a check when a message is sent successfully."

The issue is that it's not exactly this.
"Delivery" means the network has distributed the message to the final user. This is sent by the network (if I'm not wrong).
"Read" means the user has read the message. On some devices, the user may prevent this from happening. Some devices may even not support this, which means they would not send a read report at all.

> 
> "Read Report" -> "Notify when opened"
> "You'll see a check when a message you sent is opened."
> 
> 
> And on this screen -
> https://bug919974.bugzilla.mozilla.org/attachment.cgi?id=8336260:
> 
> "Message Report" -> "Message Details"
> 
> "Delivery: 05th october 2013 at 11:45" -> "Sent: 5 October 2013 at 11:45"

same comment

> 
> "Read: Report requested" -> "Opened: pending"
> 
> Hope all this is clear. Let me know if you need more feedback.

Other than that, I like this. (I'll still want Ayman to come and say "fine").


(In reply to Axel Hecht [:Pike] from comment #25)
> a) what do we do with this bug? Back out and re-land with the right strings?

Another bug please. Especially that the delivery report part landed a loooooong time ago.


Thanks to all for your dedication!

Comment 28

5 years ago
My problem is that I can't really publish the strings on gaia master like this, so this is blocking l10n of 1.4 at this point.

And I don't think that 2014-02-18 (landing date) is looooong time ago.
Axel: this bug was only about the read reports feature. The delivery report feature landed a loooooooooooong time ago (bug 933131 in Messages, I don't know for the Settings part). That's what I meant.

Since we want to change all strings, this needs another bug. We can't backout the delivery reports.

Also, a precision:

> First, a question to make sure I understand: When we say "request
> notification of delivery", do they actually receive a notification or just a
> green checkmark within the message details when they open the message/status?

Actually, my bad, Bug 933133 will implement a real notification for these. We'll probably need to have your help on the l10n strings there too.

Comment 30

5 years ago
> > "Delivery Report" -> "Notify when sent"
> > "You'll see a check when a message is sent successfully."
> 
> The issue is that it's not exactly this.
> "Delivery" means the network has distributed the message to the final user.
> This is sent by the network (if I'm not wrong).
> "Read" means the user has read the message. On some devices, the user may
> prevent this from happening. Some devices may even not support this, which
> means they would not send a read report at all.
> 

Based on your description, I still think that "Sent" and "Opened" are the right words to use to describe this to the user. However, the concept that the info won't always be available needs to be a bit clearer. I think it is difficult to make this clear within the texts describing the ON/OFF switch for reports. So, a couple suggestions:

1. If we have a way to know when a sent/opened report is requested but not available, then we should create 3 types of status in the Message Details: "Pending" (report is requested, but we don't have any answer yet), "No info available" (report was requested, but was not received), "5 October 2013 at 11:45" (report received). This way, we don't need to explain a priori that sometimes info won't be available. It will simply be clear (and a standard occurrence in lots of similar messaging contexts) that data could not be retrieved about this message.

2. If we don't have a way to distinguish reports that are requested and still pending from reports that are requested but never going to be received, it's a bit more complicated. My suggestion would be to add a note somewhere that gives a general warning that data is not always available. Maybe on the Settings screen just below the report ON/OFF options, and on the Message Details screen just below the statuses, something like: "It's not always possible to confirm when sent/opened."



In other news, and to avoid disappearing suddenly, I wanted to let you know that this will be my last response on bugzilla. My last day at Telefónica is this Friday and I've been asked to focus exclusively on other projects with my remaining time. Good luck to you all with the continued progress on FFOS!
(In reply to tyler.altes from comment #30)
> > > "Delivery Report" -> "Notify when sent"
> > > "You'll see a check when a message is sent successfully."
> > 
> > The issue is that it's not exactly this.
> > "Delivery" means the network has distributed the message to the final user.
> > This is sent by the network (if I'm not wrong).
> > "Read" means the user has read the message. On some devices, the user may
> > prevent this from happening. Some devices may even not support this, which
> > means they would not send a read report at all.
> > 
> 
> Based on your description, I still think that "Sent" and "Opened" are the
> right words to use to describe this to the user.

about "Sent" I'm a bit concerned. Let me explain. There are IMO 4 states for a message after the user presses the "sent" button:
1. pending
2. sent (ie, the network got the message)
3. delivered to the other device by the network
4. read by the receiver

Delivery report is about 3, Read report is about 4. If we use "Sent", it means the 2nd state to me.

>  However, the concept that
> the info won't always be available needs to be a bit clearer. I think it is
> difficult to make this clear within the texts describing the ON/OFF switch
> for reports. So, a couple suggestions:
> 
> 1. If we have a way to know when a sent/opened report is requested but not
> available, then we should create 3 types of status in the Message Details:
> "Pending" (report is requested, but we don't have any answer yet), "No info
> available" (report was requested, but was not received), "5 October 2013 at
> 11:45" (report received). This way, we don't need to explain a priori that
> sometimes info won't be available. It will simply be clear (and a standard
> occurrence in lots of similar messaging contexts) that data could not be
> retrieved about this message.
> 
> 2. If we don't have a way to distinguish reports that are requested and
> still pending from reports that are requested but never going to be
> received, it's a bit more complicated. My suggestion would be to add a note
> somewhere that gives a general warning that data is not always available.
> Maybe on the Settings screen just below the report ON/OFF options, and on
> the Message Details screen just below the statuses, something like: "It's
> not always possible to confirm when sent/opened."

I think we don't have a way to distinguish. I'm not sure this is that important though, but this is Ayman's call, thus I NI him.

> 
> In other news, and to avoid disappearing suddenly, I wanted to let you know
> that this will be my last response on bugzilla. My last day at Telefónica is
> this Friday and I've been asked to focus exclusively on other projects with
> my remaining time. Good luck to you all with the continued progress on FFOS!

Oh it's so sad, thanks so much for your help with us! We'll need to find someone as valuable as you, will be tough ;)
Flags: needinfo?(aymanmaat)

Comment 32

5 years ago
(In reply to Julien Wajsberg [:julienw] from comment #31)
> (In reply to tyler.altes from comment #30)
> >  However, the concept that
> > the info won't always be available needs to be a bit clearer. I think it is
> > difficult to make this clear within the texts describing the ON/OFF switch
> > for reports. So, a couple suggestions:
> > 
> > 1. If we have a way to know when a sent/opened report is requested but not
> > available, then we should create 3 types of status in the Message Details:
> > "Pending" (report is requested, but we don't have any answer yet), "No info
> > available" (report was requested, but was not received), "5 October 2013 at
> > 11:45" (report received). This way, we don't need to explain a priori that
> > sometimes info won't be available. It will simply be clear (and a standard
> > occurrence in lots of similar messaging contexts) that data could not be
> > retrieved about this message.
> > 
> > 2. If we don't have a way to distinguish reports that are requested and
> > still pending from reports that are requested but never going to be
> > received, it's a bit more complicated. My suggestion would be to add a note
> > somewhere that gives a general warning that data is not always available.
> > Maybe on the Settings screen just below the report ON/OFF options, and on
> > the Message Details screen just below the statuses, something like: "It's
> > not always possible to confirm when sent/opened."
> 
> I think we don't have a way to distinguish. I'm not sure this is that
> important though, but this is Ayman's call, thus I NI him.

no to the best of my knowledge we cannot distinguish between requests that are pending and requests that will never be recieved.  

> > In other news, and to avoid disappearing suddenly, I wanted to let you know
> > that this will be my last response on bugzilla. My last day at Telefónica is
> > this Friday and I've been asked to focus exclusively on other projects with
> > my remaining time. Good luck to you all with the continued progress on FFOS!
> 
> Oh it's so sad, thanks so much for your help with us! We'll need to find
> someone as valuable as you, will be tough ;)

:(
Flags: needinfo?(aymanmaat)
(In reply to ayman maat :maat from comment #32)

> > > 2. If we don't have a way to distinguish reports that are requested and
> > > still pending from reports that are requested but never going to be
> > > received, it's a bit more complicated. My suggestion would be to add a note
> > > somewhere that gives a general warning that data is not always available.
> > > Maybe on the Settings screen just below the report ON/OFF options, and on
> > > the Message Details screen just below the statuses, something like: "It's
> > > not always possible to confirm when sent/opened."
> > 
> > I think we don't have a way to distinguish. I'm not sure this is that
> > important though, but this is Ayman's call, thus I NI him.
> 
> no to the best of my knowledge we cannot distinguish between requests that
> are pending and requests that will never be recieved.  

Ayman, then, what do you think about Tyler's suggestion? do you think we need it?
Flags: needinfo?(aymanmaat)

Comment 34

5 years ago
(In reply to Julien Wajsberg [:julienw] from comment #33)
> (In reply to ayman maat :maat from comment #32)
> 
> > > > 2. If we don't have a way to distinguish reports that are requested and
> > > > still pending from reports that are requested but never going to be
> > > > received, it's a bit more complicated. My suggestion would be to add a note
> > > > somewhere that gives a general warning that data is not always available.
> > > > Maybe on the Settings screen just below the report ON/OFF options, and on
> > > > the Message Details screen just below the statuses, something like: "It's
> > > > not always possible to confirm when sent/opened."
> > > 
> > > I think we don't have a way to distinguish. I'm not sure this is that
> > > important though, but this is Ayman's call, thus I NI him.
> > 
> > no to the best of my knowledge we cannot distinguish between requests that
> > are pending and requests that will never be recieved.  
> 
> Ayman, then, what do you think about Tyler's suggestion? do you think we
> need it?

Its a good user centred proposal from Tyler, but I don't think we *need* it because other systems do not provide such guidance and i think there is a push on in Moz to reduce the amount of copy on the interfaces of the phone. However if there is desire to have such guidance, I would not argue against it.
Flags: needinfo?(aymanmaat)
(putting a NI to myself to remember to file a bug about changing the string, with a summary of this discussion)
Flags: needinfo?(felash)
Filed Bug 977081 for Settings.
Filed Bug 977088 for Messages.
Flags: needinfo?(felash)
You need to log in before you can comment on or make changes to this bug.