1180470 - [Messages] Propose a setting to disable sending the read report

Status: RESOLVED WONTFIX

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

Description

[Julien Wajsberg [:julienw]]

Currently we have no way to disable sending a read report when reading a MMS. We need a way to let the user disable this for privacy reasons.

We'll need a change in Settings and a change in SMS. The API already has the necessary argument to the "markMessageRead" method.

We need some UX for Settings.

Comment 1

[Wilfred Mathanaraj [:WDM]]

rob - i think there is different setting and perhaps we can put this in the same screen.

Comment 2

[Johan Lorenzo [:jlorenzo]]

Comms triage: Wilfred, should we put this feature in the backlog or do we add it in the 2.5 release?

Comment 3

[Rob MacDonald [:robmac]]

If this is part of 2.5, we should be able to put something together quite quickly. We'd just need clarification on the requirement. NI'ing Harly as an FYI.

Comment 4

[Harly Hsu[:harly]]

If this is required in 2.5, we will need to start to put up something quickly.
NI Morpheus as he will start to take over bugs related to comms.

Comment 5

[Wilfred Mathanaraj [:WDM]]

Updated the userstory.

Comment 6

[Morpheus Chen [:morpheus] UX]

Thanks for the information. But now I am still occupied by other project, will take over this bug within these days.

Comment 7

[Wilfred Mathanaraj [:WDM]]

 wesley since product has no right to set the feature-b2g flag, can you please set this to 2.5 blocker (plus clear the blocking-b2g).

Comment 8

[Morpheus Chen [:morpheus] UX]

Attachment: [Message] Send read report v1.0.pdf

I've created the spec for the configuration,
please find the attachment for the detailed spec.
Let me know if any questions, thanks.

Besides, 
I've activated "Send read receipt" for the iPhone 6, 
but I still couldn't get any read reports, even delivery reports too. 
could anyone help to check if it's a bug?

Comment 9

[Julien Wajsberg [:julienw]]

The feature is highly carrier dependent. The easiest way to know whether this is the case is to try with another non-Firefox OS phone.

For example delivery reports are not sent by the receiver at all, it's sent by the network. Read reports are sent by the receiver. So if you don't receive delivery reports I'd bet on a carrier issue.

Comment 10

[Morpheus Chen [:morpheus] UX]

It makes sense.
Because I tried and it works on Android phone(HTC butterfly 2).
But it doesn't work on iPhone 6,
perhaps it's indeed a carrier issue.
Thanks.

Comment 12

[Julien Wajsberg [:julienw]]

Now that there is a proper spec, the work is well-defined and because we likely don't have the resource to do it, I'd like to try putting this as a mentored bug.

I'd like somebody who knows already how to hack Gaia because there are 2 apps to change here: Settings and SMS.

In Settings we need to add the new setting as defined in attachment 8639185 [details]. This should be reasonnably easy. see [1]. We can use the setting name "messages.mms.sendReadReport.enabled".

[1] https://github.com/mozilla-b2g/gaia/blob/05b4ddbc7ee7747bd754da715c5f42a146976d4d/apps/settings/elements/messaging.html#L23-L41

In SMS the related code is currently in [1] (although this will move to [2] in the coming weeks). We'll also need to change code in the Settings object [3] (that will also move to a service in [4]). Then from [1] we can access the new property from Settings.

So in SMS there is some moving foundation, that's also why I'd like somebody already somewhat experienced to work on this.

[1] https://github.com/mozilla-b2g/gaia/blob/94767d1a26626a2e0488a6ced23150c87687c96b/apps/sms/services/js/message_manager.js#L436
[2] https://github.com/mozilla-b2g/gaia/blob/94767d1a26626a2e0488a6ced23150c87687c96b/apps/sms/services/js/conversation/conversation_service.js#L77
[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/views/shared/js/settings.js
[4] bug 1179628

Comment 13

[Julien Wajsberg [:julienw]]

I think this could be an interesting bug because it's a complete feature, albeit quite self-contained and small.

Comment 14

[kumar rishav (:rishav_)]

Hi Julien,
I would like to work on it. Assigning to myself.
Thanks

Comment 15

[kumar rishav (:rishav_)]

Hi Julien
How to deal with various panels. i.e how to get the value of Send-Read-Report switch (while passing argument in message_manager.js)? 
Thanks

Comment 16

[Julien Wajsberg [:julienw]]

Answered on IRC.

Have a look at the existing Settings object (in settings.js) as well.

Comment 17

[kumar rishav (:rishav_)]

Hi Julien,
In message_manager.js, we have two methods related to read/unread.
1. markThreadRead - > Takes list of threadId(array of threadIds) and isRead as parameter, which later call method 2 with list of message-id (array of message-Ids) and isRead as parameter.
2. markMessagesRead

But method 2 is recursive function, where each message-id is processed one by one and it's status(read/unread) is changed by this api-> this._mozMobileMessage.markMessageRead(id, isRead, true);

So, if we check the send-read-reports switch status (which is asyn call) inside this method, then it will check the send-read-reports switch status in each recursive call, which will be overhead. (Don't know how much it will effect the performance, but it will effect, i guess).

So, i think, it's better to calculate the send-read-reports switch status in method 1  and pass as an argument to method 2. (Assuming user won't change send-read-reports switch status during ongoing process. very very less chance or say neglisible).

Hope you got my question :)

Thanks

Comment 18

[kumar rishav (:rishav_)]

or even better , we can have it in inbox.js (InboxView.markReadUnread).

Further more researching, i found markMessageRead is taking sendReadReport as third argument in moz_mobile_message_shim.js (as you mentioned, code is going to move here in coming weeks)

So, we have two option, 
first: check send-read-reports switch status in method 1(markThreadRead) and pass it's value i.e sendReadReport as parameter to markMessageRead.

Second: check send-read-reports switch status in InboxView.markReadUnread and pass it's value i.e sendReadReport to markThreadRead then to markMessageRead.


Thanks

Comment 19

[Julien Wajsberg [:julienw]]

I think we should do it in markMessagesRead because it's also called directly in some places (for example when we receive a message from ConversationView -- may be the only place).

Also, in the current Settings object, we can have a mirror of the value of the setting, so you'd have a synchronous access to the value.

I think it's important that even in the future we still keep this synchronous mirror access to the value, because in some cases (especially mark several thread as read) we don't want to retrieve it repeatidly.

But even with a synchronous access to the value, it's not that simple :) Because we don't want to access the value before it's ready.

So here is what I suggest; sorry it may be somewhat complicated, so tell me if you don't understand something.

* synchronize the value in settings.js.
* make it available using a promise... the promise will be resolved only once we could get the value.
* change markMessagesRead to not be recursive anymore. The easy solution is to create an inner function in this method.
* do the Settings retrieval with promise before calling the new inner function.

Does it make sense ?

Comment 20

[kumar rishav (:rishav_)]

Hi Bevis,
I wanted to know how, markMessageRead behave internally (at Gecko n network level)

I saw this code,
http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/gonk/MobileMessageDB.jsm#4107
http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/MobileMessageManager.cpp#502

When message is sent by network to sender about message report? 
What about this case:

sendReadReport=false, unread message -> tap on it , it became read message, but no report send as it is false.  Now, change status to unread again but this time set sendReadReport=true, and tap on it.
Will it send the report to sender?

Do we send anything to the network if message has been read several times (more specific-> unread to read many times) ?

Please explain its working in brief :)

Thanks

Comment 21

[kumar rishav (:rishav_)]

Also, according to me (actually oleg :)):-

"send read report" is eventually translated into some network operation so that other party knows that you've read message, but I'm not sure that your device should notify network every time you mark your message as read. To me only first "mark as read" matters for the network, the rest are just UI/DB things... Maybe I'm completely wrong

Comment 22

[Julien Wajsberg [:julienw]]

I think the same.

Morpheus can give the definite answer though.
Hey Morpheus, here is the possible STR for this question:
1. "send read report" is disabled.
2. the user reads the message.
=> no read report sent.
3. the user enables "send read report".
4. the user marks the message as "unread" (currently, we can only mark a thread "unread", but what happens is we mark the last message "unread").
5. the user enters the thread again.
=> In our opinion we should not send the read report here, even if we mark the message "read" once again.

What do you think ?

Comment 23

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to kumar rishav (:rishav_) from comment #20)
> Hi Bevis,
> I wanted to know how, markMessageRead behave internally (at Gecko n network
> level)
> 
> I saw this code,
> http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/gonk/
> MobileMessageDB.jsm#4107
> http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/
> MobileMessageManager.cpp#502
> 
> When message is sent by network to sender about message report? 
> What about this case:
> 
> sendReadReport=false, unread message -> tap on it , it became read message,
> but no report send as it is false.  Now, change status to unread again but
> this time set sendReadReport=true, and tap on it.
> Will it send the report to sender?
Yes, it will.
> Do we send anything to the network if message has been read several times
> (more specific-> unread to read many times) ?
No when unread to read multiple times.

A read report of a message will be sent only when the following conditions are fulfilled:
1. It's an incoming mms message.
2. |aSendReadReport| flag is set.
3. The read status of this message was changed from UNREAD to READ.
4. The originator of this message has requested a read report.
5. The read report of this message was not sent yet.

https://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/gonk/MobileMessageDB.jsm#4156-4167,4190-4194


> 
> Please explain its working in brief :)
> 
> Thanks

Comment 24

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] from comment #22)
> I think the same.
> 
> Morpheus can give the definite answer though.
> Hey Morpheus, here is the possible STR for this question:
> 1. "send read report" is disabled.
> 2. the user reads the message.
> => no read report sent.
> 3. the user enables "send read report".
> 4. the user marks the message as "unread" (currently, we can only mark a
> thread "unread", but what happens is we mark the last message "unread").
> 5. the user enters the thread again.
> => In our opinion we should not send the read report here, even if we mark
> the message "read" once again.
> 
> What do you think ?

IMO, the user might want to postpone his/her response of the message to the sender later.
Otherwise, if the user replies a message before a read report is replied in advance, this might somehow breaks "the circle of trust" between the device user and the sender. (Oops!)

Comment 25

[Julien Wajsberg [:julienw]]

I don't think like you but I also think this is a minor issue. If we send a read report for an old message if the "send read report" toggle is "enabled", for this edge case, I don't think this is a big deal. So let's move forward.

Comment 26

[GitHub Autolander Bot]

Attachment: [gaia] kumarrishav:Bug-1180470 > mozilla-b2g:master

Comment 27

[kumar rishav (:rishav_)]

Comment on attachment 8664645 [details] [review]
[gaia] kumarrishav:Bug-1180470 > mozilla-b2g:master

Steve suggested to have feedback on patch , before going for unit test. [I think same here :)]

So here it is,https://github.com/mozilla-b2g/gaia/pull/31993/

Nice to have yours also :) steve

Comment 28

[Steve Chung [:steveck]]

Comment on attachment 8664645 [details] [review]
[gaia] kumarrishav:Bug-1180470 > mozilla-b2g:master

Requesting feedback from Fred for settings part.

Comment 29

[[e-mail]]

Comment on attachment 8664645 [details] [review]
[gaia] kumarrishav:Bug-1180470 > mozilla-b2g:master

looks fine but please add the default value in common-settings.json

Comment 30

[Julien Wajsberg [:julienw]]

Hey Matej, can you please have a look at [1] ? I feel the text could be made better.
See also attachment 8639185 [details] for the spec. This is the text to explain the option; if it's enabled, read reports will be sent if it's been requested by the sender; otherwise we won't send the read report even if the sender has requested it. Thanks !

[1] https://github.com/mozilla-b2g/gaia/pull/31993/files#diff-d32b8deaad757f82f46fa1f7d2c9b528R303

Comment 31

[Julien Wajsberg [:julienw]]

Comment on attachment 8664645 [details] [review]
[gaia] kumarrishav:Bug-1180470 > mozilla-b2g:master

Please better follow what I suggested in comment 19.
Otherwise good work, thanks a lot Rishav !

Comment 32

[GitHub Autolander Bot]

Attachment: [gaia] kumarrishav:new-Bug-1180470 > mozilla-b2g:master

Comment 33

[kumar rishav (:rishav_)]

Comment on attachment 8665788 [details] [review]
[gaia] kumarrishav:new-Bug-1180470 > mozilla-b2g:master

Hey Julienw,

Updated patch (actually new one, ignore the previous).
Except this nit, all other are probably fixed. This is will be done later at last because this will effect other code (marionette test), so will do at last.
 <li id="menuItem-readReport" aria-disabled="true">

Please give your feedback on it. 
(Now i understood how this one is better than previous, no unnecessary need of creating Lock every time, instead we have addobserver.) 

Thanks

Comment 34

[Julien Wajsberg [:julienw]]

Comment on attachment 8665788 [details] [review]
[gaia] kumarrishav:new-Bug-1180470 > mozilla-b2g:master

It's much closer to what I proposed, but not ready yet :)

I left more comments on github.

Good work !

Comment 35

[kumar rishav (:rishav_)]

Oh beauty, Julienw :D
what a clean, simple and small code you wrote. When i will learn to write like this :(

Though you wrote almost all the code. still give feedback :) to proceed further.

Thanks for this :) Approach to think in better and simple way :)

Comment 36

[kumar rishav (:rishav_)]

Comment on attachment 8665788 [details] [review]
[gaia] kumarrishav:new-Bug-1180470 > mozilla-b2g:master

I have added some comment on github.

Comment 37

[Morpheus Chen [:morpheus] UX]

I just had a business trip last week. Sorry for my late reply. I will review the bug tomorrow, thanks for your understanding and patience.

Comment 38

[Morpheus Chen [:morpheus] UX]

(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #22)
> I think the same.
> 
> Morpheus can give the definite answer though.
> Hey Morpheus, here is the possible STR for this question:
> 1. "send read report" is disabled.
> 2. the user reads the message.
> => no read report sent.
> 3. the user enables "send read report".
> 4. the user marks the message as "unread" (currently, we can only mark a
> thread "unread", but what happens is we mark the last message "unread").
> 5. the user enters the thread again.
> => In our opinion we should not send the read report here, even if we mark
> the message "read" once again.
> 
> What do you think ?

I agree with you. The action for "unread" is triggered by receiver, so the information for sender is useless and it might conflict with the previous report, for example, sender might get 2 read reports even though no new message sent. So, please don't send the read report even if user mark the message "read" once again.

Comment 39

[kumar rishav (:rishav_)]

Hi Morpheus,
I think, there won't be any case for 2 read reports sent. As in gecko, it once messageRecord.isReadReportSent gets true , it won't send report again.
https://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/gonk/MobileMessageDB.jsm#4156-4167,4190-4194

If i am not wrong, what Julien asked is, if we don't want to send readReport to sender (disable sendReadReport) and later if we enable it and it shouldn't send the readReport again. (Assuming user marked it unread again, then going for read action) .

Hope you got me.
Thanks

Comment 40

[Julien Wajsberg [:julienw]]

My question was specifically for the case that we haven't sent a read report yet (because it was disabled when the user _read_ the thread).

The question is quite related to privacy too (this is also where bug 1207062 came from, to give a little more context): maybe the user disabled the "send read report" option purposefully before entering a thread to read the message.

Comment 41

[Morpheus Chen [:morpheus] UX]

Hi Kumar,

Thanks for your clarification. I am just afraid there might be this situation, if it's not gonna happen, it's definitely better. : )

Here I try to sort out the rules below to make sure no misunderstanding:

1. Read report Off

Don't send any report

2. Read report Off > Read report On

Send report if any message is read 

3. Read report Off > User reads message A > Read report On

Don't sent read report for message A

4. Read report Off > User reads message A > Read report On > User marks message A "unread" > User reads message A again

Don't send read report for message A

Please let me know if any misunderstandings, thanks.

Comment 42

[Julien Wajsberg [:julienw]]

This is really clear, thanks Morpheus.

Rishav, will you please file a separate bug for Bevis here? In the RIL component.

Comment 43

[Morpheus Chen [:morpheus] UX]

(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #40)
> My question was specifically for the case that we haven't sent a read report
> yet (because it was disabled when the user _read_ the thread).
> 
> The question is quite related to privacy too (this is also where bug 1207062
> came from, to give a little more context): maybe the user disabled the "send
> read report" option purposefully before entering a thread to read the
> message.


I got what you mean, and please find the answer in my response to Kumar. 
Generally speaking, when read report On, it only affects any unread messages which are received after turning on.

Comment 44

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #42)
> This is really clear, thanks Morpheus.
> 
> Rishav, will you please file a separate bug for Bevis here? In the RIL
> component.

Bug 1209891 has been filed for this.

Comment 45

[Matej Novak [:matej]]

(In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #30)
> Hey Matej, can you please have a look at [1] ? I feel the text could be made
> better.
> See also attachment 8639185 [details] for the spec. This is the text to
> explain the option; if it's enabled, read reports will be sent if it's been
> requested by the sender; otherwise we won't send the read report even if the
> sender has requested it. Thanks !
> 
> [1]
> https://github.com/mozilla-b2g/gaia/pull/31993/files#diff-
> d32b8deaad757f82f46fa1f7d2c9b528R303

Sorry for the delayed response. I was on PTO.

Can you let me know which string or strings in particular you need me to review?

Comment 46

[Steve Chung [:steveck]]

Comment on attachment 8664645 [details] [review]
[gaia] kumarrishav:Bug-1180470 > mozilla-b2g:master

Sorry for the late reply. I think you already worked on another patch? Just left some comments in another patch.

Comment 47

[Oleg Zasypkin [:azasypkin]]

(In reply to Matej Novak [:matej] from comment #45)
> (In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #30)
> > Hey Matej, can you please have a look at [1] ? I feel the text could be made
> > better.
> > See also attachment 8639185 [details] for the spec. This is the text to
> > explain the option; if it's enabled, read reports will be sent if it's been
> > requested by the sender; otherwise we won't send the read report even if the
> > sender has requested it. Thanks !
> > 
> > [1]
> > https://github.com/mozilla-b2g/gaia/pull/31993/files#diff-
> > d32b8deaad757f82f46fa1f7d2c9b528R303
> 
> Sorry for the delayed response. I was on PTO.
> 
> Can you let me know which string or strings in particular you need me to
> review?

Hey Matej,

I believe Julien meant "Send read report when others request" string.

And from Julien's comment - "This is the text to explain the option; if it's enabled, read reports will be sent if it's been requested by the sender; otherwise we won't send the read report even if the sender has requested it."

See also attachment 8639185 [details] for the spec (page #7).

Thanks!

Comment 48

[kumar rishav (:rishav_)]

(In reply to Steve Chung [:steveck] from comment #46)
> Comment on attachment 8664645 [details] [review]
> [gaia] kumarrishav:Bug-1180470 > mozilla-b2g:master
> 
> Sorry for the late reply. I think you already worked on another patch? Just
> left some comments in another patch.

Yeah, that patch is obsolete .

Comment 49

[kumar rishav (:rishav_)]

Hey Julienw, 
I had discussion with steve on it.
Why we not returning value instead of promise. If we go for promise, then we could have get the value using mozSetting.lock() instead.

Also, i think we are safe here to go without promise and return value (as you told previously, navigation is promise based and init methods are completed before any other method .

If we return the promise it didn't make huge difference compare to getting value using mozSettings.lock()

Thanks

Comment 50

[kumar rishav (:rishav_)]

Clearing ni? got clear with steve :)

Comment 51

[Matej Novak [:matej]]

(In reply to Oleg Zasypkin [:azasypkin][⏰UTC+1] from comment #47)
> (In reply to Matej Novak [:matej] from comment #45)
> > (In reply to Julien Wajsberg [:julienw] (away Oct 1st, 2nd) from comment #30)
> > > Hey Matej, can you please have a look at [1] ? I feel the text could be made
> > > better.
> > > See also attachment 8639185 [details] for the spec. This is the text to
> > > explain the option; if it's enabled, read reports will be sent if it's been
> > > requested by the sender; otherwise we won't send the read report even if the
> > > sender has requested it. Thanks !
> > > 
> > > [1]
> > > https://github.com/mozilla-b2g/gaia/pull/31993/files#diff-
> > > d32b8deaad757f82f46fa1f7d2c9b528R303
> > 
> > Sorry for the delayed response. I was on PTO.
> > 
> > Can you let me know which string or strings in particular you need me to
> > review?
> 
> Hey Matej,
> 
> I believe Julien meant "Send read report when others request" string.
> 
> And from Julien's comment - "This is the text to explain the option; if it's
> enabled, read reports will be sent if it's been requested by the sender;
> otherwise we won't send the read report even if the sender has requested it."
> 
> See also attachment 8639185 [details] for the spec (page #7).
> 
> Thanks!

I would suggest "Send read reports only when requested" to make it clear that otherwise they won't be sent.

How does that sound?

Comment 52

[kumar rishav (:rishav_)]

I personally feel previous one better than this one, IMO :) 
OR
What about this:
'Send read report for each message received'  
or
'Send read report for each message read'
or
'Send read report for each message received when others request' 
or
'Send read report for each message read when others request' 

Thanks

Comment 53

[Julien Wajsberg [:julienw]]

(In reply to Matej Novak [:matej] from comment #51)

> I would suggest "Send read reports only when requested" to make it clear
> that otherwise they won't be sent.
> 
> How does that sound?

mmm for me, it means that if the option is disabled, we'll send read reports even if not requested.

Maybe removing the "only"? "Send read reports when requested"
Or maybe reverse the setting meaning and use: "Don't send read reports even when requested". This makes it clear we're trying to save his privacy.

What do you think ?

Comment 54

[Matej Novak [:matej]]

(In reply to Julien Wajsberg [:julienw] from comment #53)
> (In reply to Matej Novak [:matej] from comment #51)
> 
> > I would suggest "Send read reports only when requested" to make it clear
> > that otherwise they won't be sent.
> > 
> > How does that sound?
> 
> mmm for me, it means that if the option is disabled, we'll send read reports
> even if not requested.
> 
> Maybe removing the "only"? "Send read reports when requested"
> Or maybe reverse the setting meaning and use: "Don't send read reports even
> when requested". This makes it clear we're trying to save his privacy.
> 
> What do you think ?

Are there three states of this?

1. Always send
2. Send when requested
3. Never send

Or just two?

1. Send when requested
2. Never send

Maybe it could be something like the following:

Send read reports
[] Always
[] When requested
[] Never

(Though maybe "always" isn't needed if that isn't an option.)

Comment 55

[kumar rishav (:rishav_)]

Attachment: Atg0LVVBMgpi7bk4duC-d3SRLuF8x4wB8ZbZSHhdGvjJ.jpg

Check the third one. (send Read Report)
I am thinking to adopt the second one too :D
Feels good to me

Comment 56

[Julien Wajsberg [:julienw]]

(In reply to Matej Novak [:matej] from comment #54)
> Are there three states of this?
> 
> 1. Always send
> 2. Send when requested
> 3. Never send

> 
> Or just two?
> 
> 1. Send when requested
> 2. Never send

Just two :)

> 
> Maybe it could be something like the following:
> 
> Send read reports
> [] Always
> [] When requested
> [] Never

So you mean, a select box instead of a check box ?

This could work for me but then we need Morpheus advice because this changes the UX. What do you think Morpheus ?


The text that kumar attached (I guess it's from Android ?) could work for me too.

Comment 57

[Julien Wajsberg [:julienw]]

Comment on attachment 8665788 [details] [review]
[gaia] kumarrishav:new-Bug-1180470 > mozilla-b2g:master

Left some comments on github.

Waiting for the final text, and then the tests :D

Comment 58

[kumar rishav (:rishav_)]

Yeah, It's from android. I will suggest to adopt both (for send read request and request read report)

Comment 59

[Matej Novak [:matej]]

(In reply to kumar rishav (:rishav_) from comment #58)
> Yeah, It's from android. I will suggest to adopt both (for send read request
> and request read report)

Sure. Sounds good to me.

Comment 60

[kumar rishav (:rishav_)]

Hi Matej,
I had a look on the attached UX proposal. I think it should be 'Report' not 'Reports' as only one report is send/receive for each message irrespective of report type. In above attached image, it is 'Report'. what do you say?

If yes (i.e 'Report' is better), then i will file a bug (it will be easy to track this change,as it will effect other string too) for it after your approval.  

Thanks

Comment 61

[Julien Wajsberg [:julienw]]

To make things clearer, Kumar is talking about the option title in Settings.

Comment 62

[Morpheus Chen [:morpheus] UX]

> > Maybe it could be something like the following:
> > 
> > Send read reports
> > [] Always
> > [] When requested
> > [] Never
> 
> So you mean, a select box instead of a check box ?
> 
> This could work for me but then we need Morpheus advice because this changes
> the UX. What do you think Morpheus ?
> 

From UX perspective, I don't want to make things so complicated. I suggest keeping two options since it will be easier for users to understand and control, and frankly speaking, receiver doesn't need to know whether sender requests read report or not. But the mechanism behind the setting will be like this:

When send read report on, 
if sender requests, send read report.
if sender doesn't request, don't send read report.

When send read report off,
Don't send any read report in any cases.

Comment 63

[Matej Novak [:matej]]

(In reply to kumar rishav (:rishav_) from comment #60)
> Hi Matej,
> I had a look on the attached UX proposal. I think it should be 'Report' not
> 'Reports' as only one report is send/receive for each message irrespective
> of report type. In above attached image, it is 'Report'. what do you say?
> 
> If yes (i.e 'Report' is better), then i will file a bug (it will be easy to
> track this change,as it will effect other string too) for it after your
> approval.  

There are a few attachments on this bug. Can you clarify which one you'd like me to look at? Thanks.

Comment 64

[kumar rishav (:rishav_)]

Hi matej
I am talking about the proposed UX (i.e [Message] Send read report v1.0.pdf )
Thanks

Comment 65

[Matej Novak [:matej]]

(In reply to kumar rishav (:rishav_) from comment #64)
> Hi matej
> I am talking about the proposed UX (i.e [Message] Send read report v1.0.pdf )
> Thanks

Thanks. I actually think "Reports" is fine and right in the header and "report" is better in the description, so I would leave it as is in the proposal.

Comment 66

[kumar rishav (:rishav_)]

Hi Matej,
Please have a look, if all three(3x2 = 6) strings is fine and good to go.

Delivery Reports
Request notification of delivery for each message sent

Request Read Reports
Request message read report for each message sent

Send Read Reports
Send read report if requested by the MMS sender

Thanks

Comment 67

[kumar rishav (:rishav_)]

Comment on attachment 8665788 [details] [review]
[gaia] kumarrishav:new-Bug-1180470 > mozilla-b2g:master

Hi Julienw,
Patch is also most done. Have a look. Left few comments for you on github.
Thanks

Comment 68

[Matej Novak [:matej]]

(In reply to kumar rishav (:rishav_) from comment #66)
> Hi Matej,
> Please have a look, if all three(3x2 = 6) strings is fine and good to go.
> 
> Delivery Reports

The other two titles have "request" and "send" in them, but not this one. Should it say "Request Delivery Reports"?

> Request notification of delivery for each message sent

I would make this "delivery notification" instead of "notification of delivery"

> Request Read Reports
> Request message read report for each message sent

Looks good.

> Send Read Reports
> Send read report if requested by the MMS sender

Could this just say "the sender" instead of "MMS sender"?

Comment 69

[kumar rishav (:rishav_)]

Attachment: 2015-10-16-04-00-52.png

So, this is how new Message Settings looks. :)

Comment 70

[Julien Wajsberg [:julienw]]

> Request notification of delivery for each message sent
I'd use "Request a notification of delivery for each message sent" (adding 'a').

> Request message read report for each message sent
I'd write "Request a read report for each message sent" instead (adding 'a' and remove the 'message' repetition).

> Send read report if requested by the MMS sender
I'd use "Send a read report if requested by the sender" (adding 'a').

What do you think Matej ?

Comment 71

[Matej Novak [:matej]]

(In reply to Julien Wajsberg [:julienw] (PTO -> 2016) from comment #70)
> > Request notification of delivery for each message sent
> I'd use "Request a notification of delivery for each message sent" (adding
> 'a').
> 
> > Request message read report for each message sent
> I'd write "Request a read report for each message sent" instead (adding 'a'
> and remove the 'message' repetition).
> 
> > Send read report if requested by the MMS sender
> I'd use "Send a read report if requested by the sender" (adding 'a').
> 
> What do you think Matej ?

Agree with all changes. Thanks.

Comment 72

[Julien Wajsberg [:julienw]]

Comment on attachment 8665788 [details] [review]
[gaia] kumarrishav:new-Bug-1180470 > mozilla-b2g:master

This is already a very good job, I'm very sorry for the delay, I should have looked at this earlier.

I think it's nearly ready for a final review. I left some questions because I don't understand some of your remarks.

If you want a review before January you can ask a review from Steve or Oleg :)

Thanks !

Comment 73

[kumar rishav (:rishav_)]

Left few question/doubt on github.

Comment 74

[kumar rishav (:rishav_)]

forgot to ni?

Comment 75

[Julien Wajsberg [:julienw]]

Answered on github. Thanks for the NI :)

Comment 76

[kumar rishav (:rishav_)]

updated the patch in working state (except the mock related change) :)

Just ni? for reminder. 

Thanks

Comment 77

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(

Comment 78

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(