moz prefix devicestorage

RESOLVED WONTFIX

Status

()

Core
DOM: Device Interfaces
RESOLVED WONTFIX
5 years ago
4 years ago

People

(Reporter: dougt, Assigned: smaug)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Currently we have two methods on navigator that are exposed to all web that are not prefixed:

http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/devicestorage/nsIDOMNavigatorDeviceStorage.idl#15

Jonas, dhylans, and I agreed that we are not convinced that this API is *right* for the web and would like to make significant improvements to the API before it is unprefixed.  Therefore, we agreed to prefix this API before it ship.

We should patch this on all mozilla branches including b2g18.
Note, once the patch lands, gaia will be rather broken.
Created attachment 738211 [details] [diff] [review]
patch

Patch for trunk. Passes tests locally, but I could be missing something
https://tbpl.mozilla.org/?tree=Try&rev=7d57babb7fea

I have no idea how b2g tests are run. If they rely on gaia, I could imagine
them to be rather broken.
Attachment #738211 - Flags: review?(doug.turner)
Created attachment 738246 [details] [diff] [review]
gonk fix

https://tbpl.mozilla.org/?tree=Try&rev=6b5b123470bd
https://tbpl.mozilla.org/?tree=Try
https://tbpl.mozilla.org/?tree=Try&rev=004d497fe145
Wouldn't that be better to simply have this API being exposed to only applications in B2G? I am not sure if it should be prefixed or not but it would matter less if the exposition is reduced.
I don't know. I just did what dougt asked me to do :) I don't know all the reasons why the API is currently un-prefixed and should be now prefixed.

But we could move all the stuff to be in #ifdef #MOZ_B2G

Dougt?
I think we should move everything to MOZ_B2G and maybe we could also make the API not work (even not visible?) unless the running context is an application.

I believe that if we do that, prefixing or not would be a detail.
Again, I don't know the details but I understood we want to actually remove the API soon.
So releasing it without prefix is kind of bad....but using prefix is bad too...

sicking, dougt, need some clarification here.
(Reporter)

Comment 10

5 years ago
we need these API on non-b2g for testing. :(

sicking, do you want to give up testing on desktop to be able to isolate this API only for b2g apps?
Flags: needinfo?(jonas)
Is it possible to pref-off by default on desktop and enable the pref in the test?
(In reply to Doug Turner (:dougt) from comment #10)
> we need these API on non-b2g for testing. :(

Why can't we test on B2G devices?
To me, it does not seem worthwhile to break Gaia in order to prefix. Also, it would be sad if we prefixed *anything* now that Blink has adopted a no-prefix policy citing Mozilla as precedent. I'd rather limit visibility to B2G apps unless pref has been set for testing elsewhere.
(Reporter)

Comment 14

5 years ago
Comment on attachment 738211 [details] [diff] [review]
patch

Review of attachment 738211 [details] [diff] [review]:
-----------------------------------------------------------------

I really think we should do this.  And I think we should also disable devicestorage on non-b2g.  sicking, why don't we do this now and update gaia immediately?
Attachment #738211 - Flags: review?(doug.turner) → review?(jonas)
Comment on attachment 738211 [details] [diff] [review]
patch

Review of attachment 738211 [details] [diff] [review]:
-----------------------------------------------------------------

r=me as long as you also fix this in gaia too.

I agree we should do this sooner rather than later.
Attachment #738211 - Flags: review?(jonas) → review+
Flags: needinfo?(jonas)
Dougt said someone else will take care of gaia. I don't know who.
Flags: needinfo?(doug.turner)
Created attachment 748795 [details] [diff] [review]
gaia patch

Is this enough?
Attachment #748795 - Flags: review?(21)
Comment on attachment 748795 [details] [diff] [review]
gaia patch

mozR+.
Attachment #748795 - Flags: review?(21) → review+
https://github.com/mozilla-b2g/gaia/pull/9748
baku, does that mean the patch to Gecko could land?
yes. We can land these 2 patches. Where are we going to land the gecko patch? b2g18?
I didn't test ALL the apps... if you don't mind we can land them in 5/6 hours, time to give additional testing.
hmm, I'm not sure where this should land. Initial comment talks about b2g18, but are we really supposed to make this kind of major changes to that branch? Doug?
(Reporter)

Comment 23

4 years ago
sad, but we're too late for this.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WONTFIX
(Reporter)

Updated

4 years ago
Flags: needinfo?(doug.turner)
You need to log in before you can comment on or make changes to this bug.