Closed Bug 1105548 Opened 5 years ago Closed 3 years ago

[Messages] Notifications for messages will still be present when a message is received while in the message thread opened from contacts

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(tracking-b2g:backlog, b2g-v1.4 affected, b2g-v2.0 affected, b2g-v2.0M affected, b2g-v2.1 affected, b2g-v2.1S affected, b2g-v2.2 affected, b2g-master affected)

RESOLVED WONTFIX
tracking-b2g backlog
Tracking Status
b2g-v1.4 --- affected
b2g-v2.0 --- affected
b2g-v2.0M --- affected
b2g-v2.1 --- affected
b2g-v2.1S --- affected
b2g-v2.2 --- affected
b2g-master --- affected

People

(Reporter: AdamA, Unassigned)

References

()

Details

(Whiteboard: [2.2-Daily-Testing][sms-papercuts])

Attachments

(2 files)

Attached file logcat
Description:
if the user goes to a contact and sends an sms from the link in the contact page they will be in the thread view for that conversation. If a text message is received on this thread view the user will still have a new message notification in the utility tray and on the lock screen if the phone is restarted.

Prerequisite:
Have a contact with a valid phone number

Repro Steps:
1) Update a Flame to 20141126040207
2) Go to Contacts
3) Enter a contact info page and choose to send a text
4) Send a text to the contact
5) Stay in the message thread
6) Receive a message from that message thread
7) Pull down utility tray
8) Observe new message notification in utility tray

Actual:
there is a notification present for a message that the user has read

Expected:
It is expected that there is no notification present for messages that have been seen

Environmental Variables:
Device: Flame 2.2 (319mb)(Full Flash)
Build ID: 20141126040207
Gaia: 41b7be7c67167f367c3c4982ff08651d55455373
Gecko: 7bcc6573d204
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Repro frequency: 5/5
See attached: video clip(http://youtu.be/OaWe-klzPG8), logcat
---------------------------------------------------------------------
This issue also occurs on 2.1 Flame, and the previous 2.2 Flame.

Environmental Variables:
Device: Flame 2.1 (319MB)(Full Flash)
BuildID: 20141126001202 (Full Flash)
Gaia: db2e84860f5a7cc334464618c6ea9e92ff82e9dd
Gecko: 211eae88f119
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 (319mb)(Full Flash)
Build ID: 20141125040209
Gaia: 824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko: acde07cb4e4d
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Result:
there is a notification present for a message that the user has read
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
QAWanted for branch checks.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: qawanted
QA Contact: ddixon
Branch Check

Issue DOES occur in Flame 2.2, 2.1, 2.0  (shallow flash, eng. build, 319 MB memory). 

Actual Results: User receives a text notification despite already having seen the text message. 

Device: Flame 2.2
BuildID: 20141201032744
Gaia: 39214fb22c203e8849aaa1c27b773eeb73212921
Gecko: 08be3008650f
Version: 37.0a1 (2.2)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
----------------------------------------------
Device: Flame 2.1
BuildID: 20141128073618
Gaia: ccb49abe412c978a4045f0c75abff534372716c4
Gecko: 18fb67530b22
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
----------------------------------------------
Device: Flame 2.0
BuildID: 20141126072826
Gaia: 8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko: c756bd8bf3c3
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
Funny that I experienced this yesterday too :)

I'd like to know if this happens also when there is no Messages app in background (if it is please forcibly close it using task manager) before you receive the message.
Flags: needinfo?(aalldredge)
Whiteboard: [2.2-Daily-Testing] → [2.2-Daily-Testing][sms-papercuts]
This issue still occurs without the message being open in the background prior to opening contacts.
Flags: needinfo?(aalldredge)
QA, can we know the result on v1.3 and v1.4 too?
Keywords: qawanted
QA Contact: ddixon → jmercado
As 1.3 and 1.4 aren't available as Flame KK I checked this on JB.  I realize that makes the testing mostly invalid but hopefully it will help.  The issue DOES reproduce on the 1.3 v123 JB base and with 1.4 Flashed on top of it.

Environmental Variables:
Device: Flame 1.4
BuildID: 20141002180522
Gaia: b06fee13c1d80bd2a463be1f2fb1d59169af9298
Gecko: ea144d59c01e
Version: 30.0 (1.4) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
Flipping tracking flag to 'affected' for 1.4 (there isn't even a 1.3 flag to flip).
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
QA Contact: jmercado
Thanks a lot Jayme, the code should not have a different behavior with different base build, so the testing is not invalid. It helps showing that we always had this issue :)
Hey Alive,

While investigating possible solutions for this bug I tried to subscribe for "sms-received" system message when app is run as inline activity (navigator.mozSetMessageHandler('sms-received', handler)), but seems that this handler is never called for inline activity. 'sms-received' system message is either handled by existing "main" application instance or if "main" instance isn't run - system runs it to handle system message.

When Messages app is run as inline-activity, entry point is "/index.html#activity-xxx" if it's useful to know.

So do you know if it's by design or some sort of bug? At first I thought it maybe related to bug 931339, but not sure.

Thanks!
Flags: needinfo?(alive)
(In reply to Oleg Zasypkin [:azasypkin] from comment #9)
> Hey Alive,
> 
> While investigating possible solutions for this bug I tried to subscribe for
> "sms-received" system message when app is run as inline activity

This sounds a gecko bug, ni Fabrice - inline activity cannot get system message.

> (navigator.mozSetMessageHandler('sms-received', handler)), but seems that
> this handler is never called for inline activity. 'sms-received' system
> message is either handled by existing "main" application instance or if
> "main" instance isn't run - system runs it to handle system message.
> 
> When Messages app is run as inline-activity, entry point is
> "/index.html#activity-xxx" if it's useful to know.

Okay this might be the problem - gecko does not identify the hash here. It only see index.html and does not invoke your system message handler in index.html#xxxxx

> 
> So do you know if it's by design or some sort of bug? At first I thought it
> maybe related to bug 931339, but not sure.
> 
> Thanks!

I tend to think this is a system message design issue. And I am afraid we have no way to workaround it in gaia side.
Flags: needinfo?(alive) → needinfo?(fabrice)
Experimental WIP that uses InterInstanceEventDispatcher (SharedWorker) to query information (visible or not, current panel) about other active Messages app instances.
Duplicate of this bug: 1116671
Comment on attachment 8544424 [details] [review]
GitHub pull request URL (experimental)

Hey guys,

It's kind of experimental and a bit overcomplicated patch without much optimizations. But I'd be happy to get your early (but not urgent at all) feedback about the idea in general.

This patch relies on the fact that only "main" instance can get "sms-received" system message.

Known issue: when message is received while in Thread opened from Contacts, user will hear only sound signal, for some reason vibration doesn't work - will dig into it if we stick with this idea. 

Thanks!
Attachment #8544424 - Flags: feedback?(schung)
Attachment #8544424 - Flags: feedback?(felash)
(In reply to Oleg Zasypkin [:azasypkin] from comment #13)
> 
> Known issue: when message is received while in Thread opened from Contacts,
> user will hear only sound signal, for some reason vibration doesn't work -
> will dig into it if we stick with this idea. 

You need to look whether the sound is sent by the Messages app or by the System app.
If it's the Messages app, maybe we don't vibrate if the app's window is not visible right now.
In notify.js the vibration will still check the visibility of message app and only vibrate when it's in foreground. Maybe you can reuse the InterInstanceEventDispatcher in Notify.vibrate or simply add "force" parameter for vibrate.
(In reply to Steve Chung [:steveck] from comment #15)
> In notify.js the vibration will still check the visibility of message app
> and only vibrate when it's in foreground. 

Right! It seems when app is hidden vibration is handled by system(?) when notification is created.

> Maybe you can reuse the InterInstanceEventDispatcher in Notify.vibrate or simply add "force"
> parameter for vibrate.

Yep, sounds like a plan :) Will try.
Okay, hidden document can't call vibrate per spec [1] - "If the hidden attribute [PAGE-VISIBILITY] is set to true, then return false and terminate these steps.". 

But there is note about privileged apps - "A trusted (also known as privileged) application that integrates closely with the operating system's functionality may vibrate the device even if such an application is not visible at all, and thus may ignore the previous step. "

Looking at implementation at [2], looks like we don't respect this note.

I'll think how to overcome this.

[1] http://www.w3.org/TR/vibration/
[2] http://mxr.mozilla.org/mozilla-central/source/dom/base/Navigator.cpp#795
(In reply to Oleg Zasypkin [:azasypkin] from comment #17)
> Okay, hidden document can't call vibrate per spec [1] - "If the hidden
> attribute [PAGE-VISIBILITY] is set to true, then return false and terminate
> these steps.". 
> 
> But there is note about privileged apps - "A trusted (also known as
> privileged) application that integrates closely with the operating system's
> functionality may vibrate the device even if such an application is not
> visible at all, and thus may ignore the previous step. "
> 
> Looking at implementation at [2], looks like we don't respect this note.
> 

Is it worth filing a separate bug and asking the DOM API peers?
(In reply to Julien Wajsberg [:julienw] (PTO until 1/5) from comment #18)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #17)
> > Okay, hidden document can't call vibrate per spec [1] - "If the hidden
> > attribute [PAGE-VISIBILITY] is set to true, then return false and terminate
> > these steps.". 
> > 
> > But there is note about privileged apps - "A trusted (also known as
> > privileged) application that integrates closely with the operating system's
> > functionality may vibrate the device even if such an application is not
> > visible at all, and thus may ignore the previous step. "
> > 
> > Looking at implementation at [2], looks like we don't respect this note.
> > 
> 
> Is it worth filing a separate bug and asking the DOM API peers?

Yep, let's try, filed a bug 1121376
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #10)
> 
> I tend to think this is a system message design issue. And I am afraid we
> have no way to workaround it in gaia side.

The relevant part of the sms manifest is:
"activities": {
    "new": {
      "href": "/index.html#activity-new",
      "filters": {
        "type": "websms/sms",
        "number": {
          "pattern":"[\\w\\s+#*().-]{0,50}"
        }
       },
      "disposition": "inline",
      "returnValue": true
    },
    "share": {
      "href": "/index.html#activity-share",
      "filters": {
        "type": ["image/*", "audio/*", "video/*", "url"],
        "number": {
          "max": 5
        }
       },
      "disposition": "inline",
      "returnValue": true
    }
  },
  "messages": [
     { "alarm": "/index.html" },
     { "sms-received": "/index.html" },
     { "notification": "/index.html#notification" }
  ],

So the system message is routed properly to /index.html only. Note that the notification one would be routed to /index.html#notification. In your case it seems that you would like it to be routed to a set of possible targets?
I guess we could do something like:
{ "sms-received": ["/index.html", "/index.html#activity-new", "/index.html#activity-share"] }
we would then try to send the message to the first already opened target, and fallback to the first entry if none of the urls are already opened.
Flags: needinfo?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #20)
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #10)
> > 
> > I tend to think this is a system message design issue. And I am afraid we
> > have no way to workaround it in gaia side.
> 
> The relevant part of the sms manifest is:
> "activities": {
>     "new": {
>       "href": "/index.html#activity-new",
>       "filters": {
>         "type": "websms/sms",
>         "number": {
>           "pattern":"[\\w\\s+#*().-]{0,50}"
>         }
>        },
>       "disposition": "inline",
>       "returnValue": true
>     },
>     "share": {
>       "href": "/index.html#activity-share",
>       "filters": {
>         "type": ["image/*", "audio/*", "video/*", "url"],
>         "number": {
>           "max": 5
>         }
>        },
>       "disposition": "inline",
>       "returnValue": true
>     }
>   },
>   "messages": [
>      { "alarm": "/index.html" },
>      { "sms-received": "/index.html" },
>      { "notification": "/index.html#notification" }
>   ],
> 
> So the system message is routed properly to /index.html only. Note that the
> notification one would be routed to /index.html#notification. 

So does that mean that when start changing app URL hashes for different panels to support app recovering from the last known URL, we won't receive system messages registered for hash-less URL? Like if we have app://sms.gaiamobile.org/index.html#composer and "sms-received" is registered for "/index.html"? That would be quite ugly to enumerate all possible app sub-links in manifest.

> In your case it seems that you would like it to be routed to a set of possible targets?
> I guess we could do something like:
> { "sms-received": ["/index.html", "/index.html#activity-new",
> "/index.html#activity-share"] }
> we would then try to send the message to the first already opened target,
> and fallback to the first entry if none of the urls are already opened.

That doesn't seem to work (system message is just ignored entirely). Was it just a guess or it should work and I see a bug? And what's the rationale behind handling system message by first "target" only and not by every or at least first-visible (eg. we'd like to call navigator.vibrate on sms-received, but it doesn't work for app that is hidden)? Of course we can workaround that with SharedWorker\BroadcastChannel (once it's landed), but IMO it's not the best thing to do.
Flags: needinfo?(fabrice)
Related issues:
* bug 818000 (see esp comment 33)
* bug 931339
(In reply to Oleg Zasypkin [:azasypkin] from comment #21)

> > In your case it seems that you would like it to be routed to a set of possible targets?
> > I guess we could do something like:
> > { "sms-received": ["/index.html", "/index.html#activity-new",
> > "/index.html#activity-share"] }
> > we would then try to send the message to the first already opened target,
> > and fallback to the first entry if none of the urls are already opened.
> 
> That doesn't seem to work (system message is just ignored entirely). Was it
> just a guess or it should work and I see a bug? And what's the rationale
> behind handling system message by first "target" only and not by every or at
> least first-visible (eg. we'd like to call navigator.vibrate on
> sms-received, but it doesn't work for app that is hidden)? Of course we can
> workaround that with SharedWorker\BroadcastChannel (once it's landed), but
> IMO it's not the best thing to do.

That was a proposal to modify our current implementation. Of course that doesn't work today at all.
If we need more changes to the way we dispatch system messages, we need to discuss that on dev-webapi, not in a bug.
Flags: needinfo?(fabrice)
Okay, thanks Fabrice!

I'd say we can try first to workaround that in Messages app to have more knowledge about real use cases before we make any good proposal to dev-webapi.
Finally updated experimental PR so that "vibration" issue is fixed too :)
See Also: → 1116671
Still need to fix issue in earliest possible. nominate to 2.2?
blocking-b2g: --- → 2.2?
tracking-b2g: --- → +
Blocks: 1116671
Assigning Oleg as he worked on some patch.

Still I'd rather take time to think of a proper solution instead of rushing a workaround. We lived with this for a long time and I don't think this really should be a blocker.
Assignee: nobody → azasypkin
triage: non-blocker, but please ask for approval to uplift.
blocking-b2g: 2.2? → backlog
Not a blocker anymore, so removing this bug from my priority-list for now. We can get back to it later if needed.
Assignee: azasypkin → nobody
Attachment #8544424 - Flags: feedback?(schung)
Attachment #8544424 - Flags: feedback?(felash)
blocking-b2g: backlog → ---
Mass closing of Gaia::SMS bugs. End of an era :(
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX
Mass closing of Gaia::SMS bugs. End of an era :(
You need to log in before you can comment on or make changes to this bug.