Closed Bug 1399038 Opened 2 years ago Closed 2 years ago

Enable Storage API on beta and release

Categories

(Core :: Storage: Quota Manager, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: shawnjohnjr, Assigned: shawnjohnjr)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 5 obsolete files)

Based on signed-off report, we can ship Storage API conditionally.
Assignee: nobody → shuang
Priority: -- → P1
One thing to be noted:
test_interfaces.html related code shall be removed.

Bug 1304966 - Mark Storage APIs as nightly-only r=bz a=me CLOSED TREE
Comment on attachment 8907451 [details] [diff] [review]
Bug 1399038 - Enable Storage API on beta/release

Request review for removing nightly-only preference.
Attachment #8907451 - Flags: review?(jvarga)
Comment on attachment 8907451 [details] [diff] [review]
Bug 1399038 - Enable Storage API on beta/release

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

::: modules/libpref/init/all.js
@@ +5815,5 @@
>  // Please note that manually entering a data: URI in the
>  // URL-Bar will not be blocked when flipping this pref.
>  pref("security.data_uri.block_toplevel_data_uri_navigations", false);
>  
> +// Enable Storage API

Is it worth to mention in the comment that it's enabled on all systems except Android ?

@@ -5821,3 @@
>  pref("dom.storageManager.enabled", true);
> -#else
> -pref("dom.storageManager.enabled", false);

Should we keep the "false" variant for android ?
Attachment #8907451 - Flags: review?(jvarga) → review+
See Also: → 1399398
(In reply to Jan Varga [:janv] from comment #10)
> Comment on attachment 8907451 [details] [diff] [review]
> Bug 1399038 - Enable Storage API on beta/release
> 
> Review of attachment 8907451 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: modules/libpref/init/all.js
> @@ +5815,5 @@
> >  // Please note that manually entering a data: URI in the
> >  // URL-Bar will not be blocked when flipping this pref.
> >  pref("security.data_uri.block_toplevel_data_uri_navigations", false);
> >  
> > +// Enable Storage API
> 
> Is it worth to mention in the comment that it's enabled on all systems
> except Android ?
Yes. We should comment it better.
> 
> @@ -5821,3 @@
> >  pref("dom.storageManager.enabled", true);
> > -#else
> > -pref("dom.storageManager.enabled", false);
> 
> Should we keep the "false" variant for android ?
I thought the default value is false if pref doesn't exist. But yes, I will keep false variant for android.
Comment on attachment 8907580 [details] [diff] [review]
Bug 1399038 - Enable Storage API on beta/release. r=janv

test_interfaces.js and the rest of service workers/worker tests requires dom peers to review. Remove "nightly" attribute.
Attachment #8907580 - Flags: review?(amarchesini)
Attachment #8907580 - Flags: review?(amarchesini) → review+
(In reply to Shawn Huang [:shawnjohnjr] from comment #11)
> > Should we keep the "false" variant for android ?
> I thought the default value is false if pref doesn't exist. But yes, I will
> keep false variant for android.

Yeah, the default is false in this case, but it seems the style in this pref is to not depend on the default value.
Comment on attachment 8907580 [details] [diff] [review]
Bug 1399038 - Enable Storage API on beta/release. r=janv

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

Looks good, thanks!
(In reply to Jan Varga [:janv] from comment #14)
> (In reply to Shawn Huang [:shawnjohnjr] from comment #11)
> > > Should we keep the "false" variant for android ?
> > I thought the default value is false if pref doesn't exist. But yes, I will
> > keep false variant for android.
> 
> Yeah, the default is false in this case, but it seems the style in this pref
> is to not depend on the default value.
I see. I understand the point.
Pushed by shuang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5982cfbf600
Enable Storage API on beta/release. r=janv, baku
https://hg.mozilla.org/mozilla-central/rev/b5982cfbf600
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Can we get https://developer.mozilla.org/en-US/docs/Web/API/Storage_API#Browser_compatibility updated, please? This is shipping in Firefox 57.
Flags: needinfo?(cmills)
Keywords: dev-doc-needed
(In reply to Andrew Overholt [:overholt] from comment #21)
> Can we get
> https://developer.mozilla.org/en-US/docs/Web/API/
> Storage_API#Browser_compatibility updated, please? This is shipping in
> Firefox 57.

Oops, sorry Andrew - I've got the support information all updated now, and I've added a note to the Firefox 57 rel notes

https://developer.mozilla.org/en-US/Firefox/Releases/57#New_APIs

Let me know if you need anything else
Flags: needinfo?(cmills)
You need to log in before you can comment on or make changes to this bug.