bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

[SMS] Class 0 messages should not be shown in the UI

RESOLVED WORKSFORME

Status

Firefox OS
Gaia::SMS
RESOLVED WORKSFORME
5 years ago
5 years ago

People

(Reporter: Leo, Assigned: Leo)

Tracking

unspecified
1.1 QE1 (5may)
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(b2g18+)

Details

(Whiteboard: [TD-8935])

Attachments

(1 attachment, 2 obsolete attachments)

355 bytes, text/html
Details
(Assignee)

Description

5 years ago
1. Title : Class 0 (Flash) SMS is showing in the threaded-view with out the content(message/text).
2. Precondition : SMS should be working
3. Tester's Action : Recieve class 0(Flash) sms - click on the notification 
4. Detailed Symptom (ENG.) : When we click on notification showing the message as an alert and the message is updating in the thread-view with out content.
5. Expected : Flash sms should not be updated in the message-view/threaded-view.
6.Reproducibility: Y
1)Frequency Rate : 100%
7.Gaia Revision: 6916e18d1f72936e892543cf4a104a7d457f78ad"

Updated

5 years ago
Assignee: nobody → chulee
I don't think this is a MozRIL problem because I do see text body of a class-0 message in threaded-view. Anshul, could you double check this?
Assignee: chulee → anshulj
Flags: needinfo?(anshulj)
(Assignee)

Comment 3

5 years ago
As per our analysis in sms.js when delete operation is successful in case of normal message thread executeDeletion() is called. In this function renderThreads() is called for updating threads.But in case of class-0 message message is deleted and then notification is shown. I am unable to see any code, which refreshes the threads. I mean there is no call for renderThreads() function. This can be a reason for no thread refresh when class-0 message is arrived. Any comments?

Comment 4

5 years ago
Alhad, please have a look at this issue.
Flags: needinfo?(anshulj) → needinfo?(alhadp)

Comment 5

5 years ago
I think what the test is saying is that Class 0 SMS messages should NOT be updated in the UI conversation view when you click on the notification. Neither QCRIL nor MozRIL stores the SMS in the database, but both generate a notification to Gaia. And the way Gaia SMS app shows messages is in the message conversation view. So its most probably working as designed. 

If the requirement for Class 0 is that it should only be shown in the notifications bar but not in the Gaia SMS app, the Gaia app should be updated to do that, there's nothing that can be done in Gecko for either RIL.
Flags: needinfo?(alhadp)
(Assignee)

Comment 6

5 years ago
Created attachment 720622 [details]
Pointer to pull request
Attachment #720622 - Flags: review?(fbsc)
(Assignee)

Updated

5 years ago
Assignee: anshulj → leo.bugzilla.gaia
(Assignee)

Comment 7

5 years ago
Hi Borja Salguero [:borjasalguero],

Please review the patch for class-0 message and give the comments for the same.

Thanks,
leo
Flags: needinfo?(fbsc)
(Assignee)

Updated

5 years ago
Summary: [SMS] Class 0 (Flash) SMS is showing in the threaded-view with out the content. → [SMS] Class 0 messages should not be shown in the UI
(Assignee)

Comment 8

5 years ago
Created attachment 721106 [details]
Pointer to pull request

Added the change in message_manager.js
Attachment #720622 - Attachment is obsolete: true
Attachment #720622 - Flags: review?(fbsc)
Attachment #721106 - Flags: review?(fbsc)
(Assignee)

Comment 9

5 years ago
Hi Borja Salguero [:borjasalguero],

Thanks for the comments.
Checked and updated the patch with changes.

Thanks,
leo

Comment 10

5 years ago
Should this change be marked as TEF+?
blocking-b2g: --- → tef?
blocking-b2g: tef? → leo?
Not a blocker for Mozilla/chipset for v1.0.1, so this wouldn't be a blocker for v1.1 either. If a carrier/OEM disagrees, please renominate for blocking.
blocking-b2g: leo? → -
(Assignee)

Comment 12

5 years ago
Hi Borja Salguero [:borjasalguero],

Solved the Linting error.
Please review it once and let us know any modifications are needed.

Thanks,
Leo.
(Assignee)

Comment 13

5 years ago
Please review the patch.
(Assignee)

Comment 14

5 years ago
Created attachment 730058 [details]
Pointer to pull request
Attachment #721106 - Attachment is obsolete: true
Attachment #721106 - Flags: review?(fbsc)
Attachment #730058 - Flags: review?(fbsc)
(Assignee)

Comment 15

5 years ago
Hi Borja Salguero,

Reuploaded with new pull request and resolved the linting errors.
Please review the new pull request and give your comments.

Thanks,
Leo

Updated

5 years ago
status-b2g18: --- → affected
status-b2g18-v1.0.1: --- → wontfix
tracking-b2g18: --- → +

Updated

5 years ago
Duplicate of this bug: 852005
(Assignee)

Updated

5 years ago
blocking-b2g: - → leo?
(Assignee)

Updated

5 years ago
blocking-b2g: leo? → ---
(Assignee)

Updated

5 years ago
blocking-b2g: --- → leo?
leo+ as it blocks certifications.
blocking-b2g: leo? → leo+
I will test it today. Sorry for the delay, but this issue it's quite complicated to test!
Flags: needinfo?(fbsc)
(Assignee)

Updated

5 years ago
Whiteboard: [TD-8935]
Target Milestone: --- → Leo QE1 (5may)

Comment 19

5 years ago
I think there are two problems should be solved:
1. Class-0 message should come out on the screen promptly without any tapping of the
notification. But now you have to tap the notification to see a class-0 message.
2. Class-0 message should not display in the thread list UI anytime and
anywhere. However, it display in the thread list UI in sms application.
This probably will get solved when bug 858908 lands today, so stay tuned :)
I'd say it's solved now, but if someone else could try would be awesome
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: needinfo?(leo.bugzilla.gaia)
Resolution: --- → WORKSFORME
(Assignee)

Comment 22

5 years ago
@fcampo

Hi,

I have tried testing with the master code but the master code has some problems.
Notification is not showing when we receive a new sms in master code.

On which code u want me to test?

Thanks,
Flags: needinfo?(leo.bugzilla.gaia)
(In reply to leo.bugzilla.gaia from comment #22)
> @fcampo
> 
> Hi,
> 
> I have tried testing with the master code but the master code has some
> problems.
> Notification is not showing when we receive a new sms in master code.
> 
> On which code u want me to test?
> 
> Thanks,

code was commited on master (https://github.com/mozilla-b2g/gaia/commit/444b622a5c19229517a9a813ad4d6711bb130382), so master should be the one to test.
The changes made on that patch would show a notification if we receive a normal sms, but NOT showing a notification if we receive a 0-type sms. That would be showing an alert to the user, and the message shouldn't be painted in sms app under any circumstance.

If you test again and this is still wrong, please let me know to open a regression

Thanks
Bug 858908 is now fixed on all branches so you should be able to verify this on v1-train. Leo, can you test with a current build and let us know the result?
Flags: needinfo?(leo.bugzilla.gaia)
(Assignee)

Comment 25

5 years ago
Hi,

@doliver

I have checked with current master code.
There are still regression issues on master code.
please check the Bug 862298 for the regression issues.
No notification on receiving new sms.Because of this unable to test the class-0.

Thanks,
Leo
Flags: needinfo?(leo.bugzilla.gaia)
Leo, now that bug 862298 is closed and Bug 858908 is fixed on v1-train, can you check this one again to verify the fix?
Flags: needinfo?(leo.bugzilla.gaia)
(Assignee)

Comment 29

5 years ago
Hi Dylan,

I have tested this scenario in v1-train.
It's working fine.

Thanks,
Leo
Flags: needinfo?(leo.bugzilla.gaia)

Comment 30

5 years ago
I have tested this pr. It's working.
blocking-b2g: leo+ → ---
status-b2g18: affected → ---
status-b2g18-v1.0.1: wontfix → ---
You need to log in before you can comment on or make changes to this bug.