855570 - Deprecate Audio Data API

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Matthew Gregan [:kinetik]]

Once our Web Audio implementation has MediaElementAudioSourceNode we can deprecate all of the old Audio Data API.

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: patch v0

Comment 2

[(no longer active)]

Why do we want to wait for that?  Our current Web Audio implementation should be able to do everything that the old Audio Data API does, right?

Comment 3

[(no longer active)]

Comment on attachment 730472 [details] [diff] [review]
patch v0

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

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +133,5 @@
>  # LOCALIZATION NOTE: Do not translate "X-Content-Security-Policy/Report-Only" or "Content-Security-Policy/Report-Only"
>  BothCSPHeadersPresent=This site specified both an X-Content-Security-Policy/Report-Only header and a Content-Security-Policy/Report-Only header. The X-Content-Security-Policy/Report-Only header(s) will be ignored.
>  # LOCALIZATION NOTE: Do not translate "NodeIterator" or "detach()".
>  NodeIteratorDetachWarning=Calling detach() on a NodeIterator no longer has an effect.
> +MozAudioDataWarning=The Mozilla Audio Data API is deprecated.  Please use the Web Audio API instead.

You probably want to add a LOCALIZATION NOTE here as well.

Comment 4

[(no longer active)]

Oh, and also, you should make these deprecation warnings conditional on whether the media.webaudio.enabled pref is true.

Comment 5

[Matthew Gregan [:kinetik]]

Attachment: patch v1

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #2)
> Why do we want to wait for that?  Our current Web Audio implementation
> should be able to do everything that the old Audio Data API does, right?

Don't we need MediaElementAudioSourceNode to replicate the functionality of the MozAudioAvailable event?  Or am I confused?

We don't have to wait, though.  It's probably better to discourage use of this API as soon as possible.

Comment 6

[(no longer active)]

(In reply to Matthew Gregan [:kinetik] from comment #5)
> Created attachment 732122 [details] [diff] [review]
> patch v1
> 
> (In reply to :Ehsan Akhgari (needinfo? me!) from comment #2)
> > Why do we want to wait for that?  Our current Web Audio implementation
> > should be able to do everything that the old Audio Data API does, right?
> 
> Don't we need MediaElementAudioSourceNode to replicate the functionality of
> the MozAudioAvailable event?  Or am I confused?

Nope, you're right, sorry, I ignored MozAudioAvailable!

> We don't have to wait, though.  It's probably better to discourage use of
> this API as soon as possible.

+1.  Er, make that +100.  :-)

Comment 7

[(no longer active)]

Comment on attachment 732122 [details] [diff] [review]
patch v1

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

::: dom/locales/en-US/chrome/dom/dom.properties
@@ +133,5 @@
>  # LOCALIZATION NOTE: Do not translate "X-Content-Security-Policy/Report-Only" or "Content-Security-Policy/Report-Only"
>  BothCSPHeadersPresent=This site specified both an X-Content-Security-Policy/Report-Only header and a Content-Security-Policy/Report-Only header. The X-Content-Security-Policy/Report-Only header(s) will be ignored.
>  # LOCALIZATION NOTE: Do not translate "NodeIterator" or "detach()".
>  NodeIteratorDetachWarning=Calling detach() on a NodeIterator no longer has an effect.
> +# LOCALIZATION NOTE: Do not translate "Mozilla Audio Data API" or "Web Audio API".

Nit: and

Comment 8

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1aee691b14b7

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1aee691b14b7

Comment 10

[Kohei Yoshino]

I've added this bug to the compatibility doc. Please correct the info if wrong.
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23

Comment 11

[Kohei Yoshino]

https://developer.mozilla.org/en-US/docs/Introducing_the_Audio_API_Extension

Comment 12

[(no longer active)]

(In reply to comment #10)
> I've added this bug to the compatibility doc. Please correct the info if wrong.
> https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_23

Looks good, thanks!

Comment 13

[(no longer active)]

Hmm, actually thinking more about this, the deprecation messages being hidden behind the webaudio pref means that once 23 gets into beta, those messages will be hidden (as the pref is off by default there.)  Perhaps I was mistaken to ask you to hide it behind the pref.  What do you think, Matthew?

Comment 14

[Matthew Gregan [:kinetik]]

Attachment: followup v0

I think it makes sense to start warning that these APIs are going away as soon as possible, so let's remove the pref check.

Comment 15

[Masatoshi Kimura [:emk]]

Then will beta users be warned while the alternative is not available? I don't think it makes sense.

Comment 16

[Masatoshi Kimura [:emk]]

>+MozAudioDataWarning=The Mozilla Audio Data API is deprecated.  Please use the Web Audio API instead.

At least this very confusing message should be changed because beta users cannot "use the Web Audio API instead".

Comment 17

[Matthew Gregan [:kinetik]]

Attachment: followup v1

I think we'd want to warn people away from using this API even if Web Audio wasn't (becoming) available, because the Audio Data API is buggy and we have not been fixing those bugs.

I agree that we should remove the recommendation to use the Web Audio API from the warning if we land my follow up patch.  Updated patch attached.

Comment 18

[(no longer active)]

Comment on attachment 751565 [details] [diff] [review]
followup v1

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

Actually I think it makes sense for us to tell developers to switch to Web Audio, even if it's not available on beta, since it will be available there in less than 6 weeks anyway, and we're not removing the Moz Audio API quite yet, so they don't need to have a replacement today.  Also, changing localized strings means that we probably won't be able to take the patch on beta anyway, which would suck.

Let's take your other patch.

Comment 19

[(no longer active)]

Comment on attachment 751539 [details] [diff] [review]
followup v0

This patch just adds a warning to the web developer's console when using the deprecated moz audio API, and it has a very low risk of breaking anything, plus it has baked on m-c for a while.

I'd like to take this patch on both aurora and beta to start evangelizing people away from this API as soon as possible.

Comment 20

[Matthew Gregan [:kinetik]]

Comment on attachment 732122 [details] [diff] [review]
patch v1

The initial patch only landed on 23, so if we want these warnings in beta we'll need to land this and the followup there.

Comment 21

[Matthew Gregan [:kinetik]]

followup v0 landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c06be9e2cf4f
https://hg.mozilla.org/releases/mozilla-aurora/rev/45ab407e8747

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c06be9e2cf4f

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/df58e8e3e743

With the follow-up patch folded in.

Comment 24

[Francesco Lodolo [:flod]]

To be completely honest I don't believe that breaking string freeze on all repositories, even more for a warning, is a good idea (the new string wasn't even mentioned in the approval request).

Comment 25

[(no longer active)]

Ah, I thought that the string was already on beta... :(  Given the fact that it was not, I think we should back out https://hg.mozilla.org/releases/mozilla-beta/rev/df58e8e3e743.  CCing more RelMan folks.

Comment 26

[(no longer active)]

(And sorry for my mistake here.)

Comment 27

[:Gavin Sharp [email: [e-mail]]]

We can just hardcode the English string for Aurora/Beta.

Comment 28

[Francesco Lodolo [:flod]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #27)
> We can just hardcode the English string for Aurora/Beta.

Considering the target of these warnings (developers), that would fine :-)

And the string is already on Aurora, so it's just a Beta problem
http://mxr.mozilla.org/l10n-mozilla-aurora/source/it/dom/chrome/dom/dom.properties#108

Comment 29

[(no longer active)]

OK, I can hardcode the string too, just waiting for confirmation from RelMan on what to do here.

Comment 30

[Alex Keybl [:akeybl]]

a=akeybl on hardcoding the string for beta instead of introducing new strings

Sorry I missed the strings in the patch - you gotta call these things out :)

Comment 31

[(no longer active)]

(In reply to Alex Keybl [:akeybl] from comment #30)
> Sorry I missed the strings in the patch - you gotta call these things out :)

I own the blame entirely, as I was under the impression that the patch that added the strings _is_ already on Beta.

Comment 32

[(no longer active)]

Attachment: Hardcode the string on beta

This is hackish but it's a one time fix.  Boris, do you mind giving this a quick review?  Thanks!

Comment 33

[Matthew Gregan [:kinetik]]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #31)
> (In reply to Alex Keybl [:akeybl] from comment #30)
> > Sorry I missed the strings in the patch - you gotta call these things out :)
> 
> I own the blame entirely, as I was under the impression that the patch that
> added the strings _is_ already on Beta.

I should have known better, I just haven't dealt with l10n issues on branches before and it never crossed my mind.  If I had filled out the proper approval form, it would have been explicit.  Sorry everyone!

Comment 34

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 753562 [details] [diff] [review]
Hardcode the string on beta

r=me

Comment 35

[(no longer active)]

https://hg.mozilla.org/releases/mozilla-beta/rev/e4456cef81be

Comment 36

[Kohei Yoshino]

Reflected status-firefox22 to the documents:
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_22
https://developer.mozilla.org/en-US/docs/Introducing_the_Audio_API_Extension