Closed Bug 1121376 Opened 5 years ago Closed 2 years ago

navigator.vibrate() should vibrate for privileged/certified apps even if they are hidden

Categories

(Core :: DOM: Device Interfaces, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: azasypkin, Unassigned)

References

Details

Per spec [1] if hidden document call vibrate it doesn't have any effect - "If the hidden attribute [PAGE-VISIBILITY] is set to true, then return false and terminate these steps.". 

But there is note there 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.

So are we going to support that note for certified/privileged FxOS apps?

[1] http://www.w3.org/TR/vibration/
[2] http://mxr.mozilla.org/mozilla-central/source/dom/base/Navigator.cpp#795
Hey Ehsan,

Do you think we should support "vibrate" for hidden privileged/certified apps? 

Let me give you more context on why this is an issue for us (FxOS Messages app). In bug 1105548 we have the following use case:

* Messages app is opened as inline activity from Contacts, user is located in a Thread panel;
* When we receive a new sms/mms for the active Thread we'd like to notify user with sound and vibration;
* The problem #1 is that app that's run as inline activity can't handle 'sms-received' system message (ni?'ed :fabrice on bug 1105548 to clarify if it's designed correctly);
* Because of the problem #1, System runs a brand new Messages app instance to handle 'sms-received' system message.
* And here's the magic begins :), new Messages app (it's in a hidden state) queries all other active in-activity-instances (via SharedWorker broadcast mechanism we have in Messages app):
** if there's no any visible Messages app instance at the moment, then we generate notification that produces both sound and vibration signals for us(looks like System app handles that, not sure);
** but if it finds one that's visible and have appropriate Thread opened, we shouldn't generate notification, but only produce sound and vibration signals.

Here's the problem #2 - app that is hidden would like to call "navigator.vibrate()". To workaround this issue we just "ask" (again via SharedWorker) visible instance to call "navigator.vibrate()" instead.

So as you see all this is quite complex and we'd be happy to make it simpler :)
Flags: needinfo?(ehsan)
See Also: → 1105548
I'm fine with lifting this restriction for certified apps.  The original reasoning behind the restriction is to enable people to get rid of pages that abuse this API to annoy the user by either closing the page/app or navigating away from it.  That makes me hesitate a bit to say the same about privileged apps.  I'm curious to know what Jonas thinks about this.
Flags: needinfo?(ehsan) → needinfo?(jonas)
(In reply to Oleg Zasypkin [:azasypkin] from comment #1)
> * The problem #1 is that app that's run as inline activity can't handle
> 'sms-received' system message (ni?'ed :fabrice on bug 1105548 to clarify if
> it's designed correctly);

This seems solvable. But see more below.

> * And here's the magic begins :), new Messages app (it's in a hidden state)
> queries all other active in-activity-instances (via SharedWorker broadcast
> mechanism we have in Messages app):
> ** if there's no any visible Messages app instance at the moment, then we
> generate notification that produces both sound and vibration signals for
> us(looks like System app handles that, not sure);

This part you need to do no matter what right? To know if you should create a system notification or not?

(Probably unrelated, but you will soon be able to use a BroadcastChannel rather than a SharedWorker to accomplish this.)

> ** but if it finds one that's visible and have appropriate Thread opened, we
> shouldn't generate notification, but only produce sound and vibration
> signals.

Either way you'll need to send a message to the visible thread so that it can be immediately updated? So making the navigator.vibrate() call here instead of in the originating window doesn't seem like it'll require a lot more code?


I'd really rather not have APIs behave different in privileged vs. "normal" apps. That just makes the API harder to document and understand, and makes transitioning between app types harder. We could provide special API for privileged apps, and guard it with a permission, but it seems like a bunch of work if it's not really making gaia that much simpler.
Flags: needinfo?(jonas)
(In reply to Jonas Sicking (:sicking) from comment #3)
> (In reply to Oleg Zasypkin [:azasypkin] from comment #1)
> > * The problem #1 is that app that's run as inline activity can't handle
> > 'sms-received' system message (ni?'ed :fabrice on bug 1105548 to clarify if
> > it's designed correctly);
> 
> This seems solvable. But see more below.

Yeah, trying to solve at the moment...

> 
> > * And here's the magic begins :), new Messages app (it's in a hidden state)
> > queries all other active in-activity-instances (via SharedWorker broadcast
> > mechanism we have in Messages app):
> > ** if there's no any visible Messages app instance at the moment, then we
> > generate notification that produces both sound and vibration signals for
> > us(looks like System app handles that, not sure);
> 
> This part you need to do no matter what right? To know if you should create
> a system notification or not?

Yes, we're going to do this anyway to support our multi-instance use cases, unless something exceptionally better comes from solution for problem #1.

> 
> (Probably unrelated, but you will soon be able to use a BroadcastChannel
> rather than a SharedWorker to accomplish this.)

Sure, we've been tracking this for some time already and going to migrate to BroadcastChannel API in bug 1080480 once it's landed and become more or less stable. 

> 
> > ** but if it finds one that's visible and have appropriate Thread opened, we
> > shouldn't generate notification, but only produce sound and vibration
> > signals.
> 
> Either way you'll need to send a message to the visible thread so that it
> can be immediately updated? So making the navigator.vibrate() call here
> instead of in the originating window doesn't seem like it'll require a lot
> more code?

We don't use system message for this, instead we rely on mozMobileMessage.onmessagereceived. But yeah, it doesn't require a lot of work since we already have InterInstanceEventDispatcher (wrapper around SharedWorker) that can ask visible instance to do vibration for us [1] (experimental patch in case you're curious), it just makes code more complex :) 

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

> 
> I'd really rather not have APIs behave different in privileged vs. "normal"
> apps. That just makes the API harder to document and understand, and makes
> transitioning between app types harder. We could provide special API for
> privileged apps, and guard it with a permission, but it seems like a bunch
> of work if it's not really making gaia that much simpler.

I see your point, and I'm definitely against dedicated API for this. I was only thinking about indulgence\enhancement for certified apps that's allowed by spec :)

So in the end, we (Messages app developers) can live without it and rely on our broadcasting workaround if you guys think it's better to not support this.
I definitely don't want you to have to deal with too much complexity. I think I'm still not understanding why you end up with complexity though.

As I understand it, you are currently doing the following:

1. Use a SharedWorker to track which thread(s) are currently open in a visible view.
2. When you receive a "sms-received" system message check if there's currently a visible view for the
   affected thread.
3. If there is no such visible view, create a DOM Notification (which causes a vibration+soundeffect).
4. If there is such a visible view, instead of creating a DOM Notification, send a message to the view
   by using the SharedWorker in order to have the view do a vibration+soundeffect.
5. When displaying a thread view, listen for the you receive a mozMobileMessage.onmessagereceived event.
6. When receiving a mozMobileMessage.onmessagereceived event, and the event is for a message in the
   current thread, modify the UI to display the message.

Your goal is to remove the complexity in step 4 and instead change step 4 to be

4. If there is such a visible view, play the do a vibration+soundeffect in the current window.

But otherwise your proposal doesn't change anything. Is that correct?


My suggestion is to instead completely remove step 4 and instead change step 6 to:

6'.When receiving a mozMobileMessage.onmessagereceived event, and the event is for a message in the
   current thread, modify the UI to display the message and do a vibration+soundeffect.

This seems to simplify things as much.

Is there a reason you couldn't do that?
That seems to remove all the complexity of step 4
(In reply to Jonas Sicking (:sicking) from comment #5)
> I definitely don't want you to have to deal with too much complexity. I
> think I'm still not understanding why you end up with complexity though.

First of all, thanks a lot for paying close attention to our issue!

> As I understand it, you are currently doing the following:
> 
> 1. Use a SharedWorker to track which thread(s) are currently open in a
> visible view.
> 2. When you receive a "sms-received" system message check if there's
> currently a visible view for the
>    affected thread.
> 3. If there is no such visible view, create a DOM Notification (which causes
> a vibration+soundeffect).
> 4. If there is such a visible view, instead of creating a DOM Notification,
> send a message to the view
>    by using the SharedWorker in order to have the view do a
> vibration+soundeffect.
> 5. When displaying a thread view, listen for the you receive a
> mozMobileMessage.onmessagereceived event.
> 6. When receiving a mozMobileMessage.onmessagereceived event, and the event
> is for a message in the
>    current thread, modify the UI to display the message.
> 
> Your goal is to remove the complexity in step 4 and instead change step 4 to
> be
> 
> 4. If there is such a visible view, play the do a vibration+soundeffect in
> the current window.
> 
> But otherwise your proposal doesn't change anything. Is that correct?

Yep, that's correct.

> 
> My suggestion is to instead completely remove step 4 and instead change step
> 6 to:
> 
> 6'.When receiving a mozMobileMessage.onmessagereceived event, and the event
> is for a message in the
>    current thread, modify the UI to display the message and do a
> vibration+soundeffect.
> 
> This seems to simplify things as much.
> 
> Is there a reason you couldn't do that?
> That seems to remove all the complexity of step 4

Yeah, I think it should work even better! Will try it.
Cleaning up Device Interfaces component, and mass-marking old FxOS bugs as incomplete.

If any of these bugs are still valid, please let me know.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.