If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Hide RIL DOM APIs behind prefs

RESOLVED FIXED in Firefox 26, Firefox OS v1.2

Status

()

Core
DOM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: vicamo, Assigned: vicamo)

Tracking

(Blocks: 1 bug)

unspecified
1.3 Sprint 3 - 10/25
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+, firefox25 wontfix, firefox26 fixed, firefox27 fixed, b2g-v1.2 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
+++ This bug was initially created as a clone of Bug #914182 +++

From bug 914182 comment 15, we should apply whatever method landed in bug 914182 to other RIL specific APIs like MobileConnection, Icc, MobileMessage, CellBroadcast and Voicemail.
(Assignee)

Comment 1

4 years ago
However, some of them haven't been moved to WebIDL so we may not be able to apply the technique directly.  See also bug 865403 (Cell Broadcast), bug 859764 (Mobile Message), bug 898445 (Mobile Connection), bug 904588 (Icc).
If they haven't moved to WebIDL, are they even exposed on the global?  I see no props on Window with names from comment 0.
These APIs are hidden behind |#ifdef MOZ_B2G_RIL| atm. However the b2g team are going to reduce compile-time options.
Blocks: 916605
(Assignee)

Comment 4

4 years ago
(In reply to Masatoshi Kimura [:emk] from comment #3)
> These APIs are hidden behind |#ifdef MOZ_B2G_RIL| atm. However the b2g team
> are going to reduce compile-time options.

No, that's a request from DOM peer, Mounir.  See bug 859616 comment 13:

  The Mozilla project is heading in the exact opposite direction:
  we want to reduce the number of compile-time options because those
  options are a pain to maintain.
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #4)
> (In reply to Masatoshi Kimura [:emk] from comment #3)
> > These APIs are hidden behind |#ifdef MOZ_B2G_RIL| atm. However the b2g team
> > are going to reduce compile-time options.
> 
> No, that's a request from DOM peer, Mounir.  See bug 859616 comment 13:
> 
>   The Mozilla project is heading in the exact opposite direction:
>   we want to reduce the number of compile-time options because those
>   options are a pain to maintain.

So the b2g team are going to do that, no? I don't see how the statement is inaccurate.
(Assignee)

Comment 6

4 years ago
(In reply to :Ms2ger (back and backlogged from being away) from comment #5)
> (In reply to Vicamo Yang [:vicamo][:vyang] from comment #4)
> > No, that's a request from DOM peer, Mounir.  See bug 859616 comment 13:
> 
> So the b2g team are going to do that, no? I don't see how the statement is
> inaccurate.

I just wanted to clarify that idea, to reduce compile-time options, was not originated from RIL people.  I do agree the purpose of this bug is absolutely right.

BTW, in bug 920551, Gaia people want "navigator.mozTelephony" completely disappear even from the point of view of pages with "telephony" permission.  The same thing applies to other APIs that rely on hardware modem.
(Assignee)

Updated

4 years ago
Assignee: nobody → vyang
(Assignee)

Comment 7

4 years ago
Created attachment 811039 [details] [diff] [review]
patch
(Assignee)

Comment 8

4 years ago
Comment on attachment 811039 [details] [diff] [review]
patch

This patch uses per API preference to determine the availability of each API.  We still depend on MOZ_B2G_RIL flag to hide non-EventTarget symbols.  To remove such dependency, we have to move all interfaces to WebIDL.  For now, only WebTelephony had been fully converted to WebIDL, works to Voicemail and CellBroadcast are partially done, and little progress in Icc, MobileConnection, and SMS.  See bug 898445 for mobileconnection, bug 904588 for icc, bug 859764 for sms, bug 906398 for cellbroadcast...

This patch will serve as a base stone for bug 920551, which will make MOZ_B2G_RIL an optional feature and we might not have RIL related interfaces when building B2G.

Hiding SMS is not in the scope of this bug.  Instead, in bug 859614.  SMS is quite different because it's currently built on every platform and none of the related interfaces has been moved to WebIDL.

Changes:
1) Remove override to "dom.sms.strict7BitEncoding", "dom.sms.requestStatusReport" for they have the same values with those in modules/libpref/src/init/all.js.

2) Move "dom.mms.requestStatusReport", "wap.*" into all.js as well.  Default values not overridden in B2G.

3) Add controlling preferences for CellBroadcast/Icc/MobileConnection/Voicemail.

Full try: https://tbpl.mozilla.org/?tree=Try&rev=94337eb098be
Attachment #811039 - Flags: review?(khuey)
Attachment #811039 - Flags: feedback?(VYV03354)
Attachment #811039 - Flags: feedback?(Ms2ger)
(Assignee)

Updated

4 years ago
Attachment #811039 - Attachment description: WIP → patch
Updating the summary to reflect the reality.
Summary: Hide RIL DOM APIs from regular Web pages → Hide RIL DOM APIs behind prefs
Attachment #811039 - Flags: review?(khuey) → review+
Comment on attachment 811039 [details] [diff] [review]
patch

::: dom/tests/mochitest/general/test_interfaces.html
>+               (entry.pref && !prefs.getBoolPref(entry.pref))) {

I would not like to test the availability based on prefs. See bug 906432 comment #8.
I still think these interfaces should be visible only if the corresponding permission is granted.
I had to introduce a pref for Telephony because SpecialPowers.pushPermissions() chouldn't change the availabliity dynamically. But permissions will not be changed dynamically outside the test harness. It is a bit silly to introduce a pref to avoid the limitation of the test.
> because SpecialPowers.pushPermissions() chouldn't change the availabliity dynamically. 

Uh... it should work.  You just have to create a new iframe after the pushPermissions() call is done.
(In reply to Boris Zbarsky [:bz] from comment #12)
> > because SpecialPowers.pushPermissions() chouldn't change the availabliity dynamically. 
> 
> Uh... it should work.  You just have to create a new iframe after the
> pushPermissions() call is done.

Yeah, I could fix the mochitest using an iframe, but it had no effect on marionette somehow.
Should I file a bug about that?
(Assignee)

Comment 14

4 years ago
(In reply to Masatoshi Kimura [:emk] from comment #11)
> I still think these interfaces should be visible only if the corresponding
> permission is granted.
> I had to introduce a pref for Telephony because
> SpecialPowers.pushPermissions() chouldn't change the availabliity
> dynamically. But permissions will not be changed dynamically outside the
> test harness. It is a bit silly to introduce a pref to avoid the limitation
> of the test.

I do agree a permission based check makes sense, but yet there is a problem that we want to disable all RIL related interfaces on platforms/devices that do not have RIL functions at all.  This should also apply to apps with required permissions.  So, checking whether the interfaces are built (by prefs) and whether a certain app is granted with access to the interfaces are both necessary here.  In this bug we have the former, and probably with some fixes to Marionette, we have the latter.  What do you think?
Blocks: 920551
Comment on attachment 811039 [details] [diff] [review]
patch

OK, but please add "b2g: true" to make sure that these APIs are disabled on non-b2g. An example:
    {name: "Telephony", b2g: true, pref: "dom.telephony.enabled"},
Attachment #811039 - Flags: feedback?(VYV03354) → feedback+
(Assignee)

Comment 16

4 years ago
Nominate for koi+ because we'll need this to disable WebIDL interfaces in bug 920551, which is a koi+.
blocking-b2g: --- → koi?
(Assignee)

Comment 17

4 years ago
Created attachment 818504 [details] [diff] [review]
patch : v2

Address comment 15, add "b2g: true" back.  Re-run Mochitest https://tbpl.mozilla.org/?tree=Try&rev=bf30cbe6a793
Attachment #811039 - Attachment is obsolete: true
Attachment #811039 - Flags: feedback?(Ms2ger)
Attachment #818504 - Flags: review+
Attachment #818504 - Flags: feedback+
(Assignee)

Updated

4 years ago
Duplicate of this bug: 908681
(Assignee)

Updated

4 years ago
Target Milestone: --- → 1.3 Sprint 3 - 10/25
(Assignee)

Comment 19

4 years ago
https://hg.mozilla.org/integration/b2g-inbound/rev/125d25e3662e
https://hg.mozilla.org/mozilla-central/rev/125d25e3662e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Blocks a blocker
blocking-b2g: koi? → koi+
[/c/src-gecko/aurora]$ transplant 125d25e3662e
searching for changes
applying 125d25e3662e
patching file b2g/app/b2g.js
Hunk #1 FAILED at 382
Hunk #4 FAILED at 813
2 out of 4 hunks FAILED -- saving rejects to file b2g/app/b2g.js.rej
patching file dom/tests/mochitest/general/test_interfaces.html
Hunk #1 FAILED at 94
Hunk #2 FAILED at 141
Hunk #3 FAILED at 271
Hunk #7 FAILED at 594
Hunk #8 FAILED at 611
5 out of 8 hunks FAILED -- saving rejects to file dom/tests/mochitest/general/test_interfaces.html.rej
patching file dom/webidl/MozCellBroadcast.webidl
Hunk #1 succeeded at 5 with fuzz 2 (offset 0 lines).
patching file modules/libpref/src/init/all.js
Hunk #3 FAILED at 4448
1 out of 3 hunks FAILED -- saving rejects to file modules/libpref/src/init/all.js.rej
patch failed to apply

Updated

4 years ago
Whiteboard: [needs-aurora-patch]
(Assignee)

Comment 23

4 years ago
Created attachment 822778 [details] [diff] [review]
[aurora] patch
(Assignee)

Updated

4 years ago
Attachment #822778 - Attachment description: [b2g26_v1_2] patch → [aurora] patch
(Assignee)

Comment 24

4 years ago
try before landing: https://tbpl.mozilla.org/?tree=Try&rev=33f6aef13c56
Whiteboard: [needs-aurora-patch]
https://hg.mozilla.org/releases/mozilla-aurora/rev/60c2427efbca
status-b2g-v1.2: --- → fixed
status-firefox25: --- → wontfix
status-firefox26: --- → fixed
status-firefox27: --- → fixed
Depends on: 949887
You need to log in before you can comment on or make changes to this bug.