Closed
Bug 915604
Opened 11 years ago
Closed 11 years ago
Hide RIL DOM APIs behind prefs
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
People
(Reporter: vicamo, Assigned: vicamo)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
vicamo
:
review+
vicamo
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
+++ 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•11 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).
Comment 2•11 years ago
|
||
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.
Comment 3•11 years ago
|
||
These APIs are hidden behind |#ifdef MOZ_B2G_RIL| atm. However the b2g team are going to reduce compile-time options.
Assignee | ||
Comment 4•11 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.
Comment 5•11 years ago
|
||
(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•11 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•11 years ago
|
Assignee: nobody → vyang
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Comment 8•11 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•11 years ago
|
Attachment #811039 -
Attachment description: WIP → patch
Comment 9•11 years ago
|
||
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 10•11 years ago
|
||
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.
Comment 11•11 years ago
|
||
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.
Comment 12•11 years ago
|
||
> 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.
Comment 13•11 years ago
|
||
(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•11 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 15•11 years ago
|
||
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•11 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•11 years ago
|
||
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•11 years ago
|
Target Milestone: --- → 1.3 Sprint 3 - 10/25
Comment 20•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/125d25e3662e
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 22•11 years ago
|
||
[/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•11 years ago
|
Whiteboard: [needs-aurora-patch]
Assignee | ||
Comment 23•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #822778 -
Attachment description: [b2g26_v1_2] patch → [aurora] patch
Assignee | ||
Comment 24•11 years ago
|
||
try before landing: https://tbpl.mozilla.org/?tree=Try&rev=33f6aef13c56
Whiteboard: [needs-aurora-patch]
Comment 25•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/60c2427efbca
status-b2g-v1.2:
--- → fixed
status-firefox25:
--- → wontfix
status-firefox26:
--- → fixed
status-firefox27:
--- → fixed
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•