884407 - Update SpeechRecognitionError to use enum

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

See https://dvcs.w3.org/hg/speech-api/raw-file/tip/speechapi.html#speechreco-error

Comment 1

[Guilherme Gonçalves [:ggp]]

Attachment: Bug 884407 - Use an enum in SpeechRecognitionError

This patch creates an enum for the error codes for SpeechRecognitionError. It also removes nsIDOMSpeechRecognitionError, since that turned out to be possible (but let me know if you'd actually like to keep it), and updates the tests accordingly.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 766141 [details] [diff] [review]
Bug 884407 - Use an enum in SpeechRecognitionError


>+DOMCI_DATA(SpeechRecognitionError, mozilla::dom::SpeechRecognitionError)
No need for DOMCI_DATA


>+namespace mozilla {
>+namespace dom {
>+
>+SpeechRecognitionError::SpeechRecognitionError(mozilla::dom::EventTarget* aOwner, nsPresContext* aPresContext, nsEvent* aEvent)
>+: nsDOMEvent(aOwner, aPresContext, aEvent),
>+  mError(),
>+  mMessage()
There shouldn't be need for mMessage(). 
(codegen may create such).



>+{
>+  SetIsDOMBinding();
No need for this anymore. These days all the events use DOM bindings, so 
SetIsBOMBinding() is called in the nsDOMEvent ctor.

>+  nsresult
>+  InitSpeechRecognitionError(const nsAString& aType,
>+                             bool aCanBubble,
>+                             bool aCancelable,
>+                             SpeechRecognitionErrorCode aError,
>+                             const nsAString& aMessage);
So if we don't have .idl, why do we need this version?
Wouldn't the one with ErrorResult be enough?

Comment 3

[Guilherme Gonçalves [:ggp]]

Attachment: Use an enum in SpeechRecognitionError

Refreshing the patch after addressing review comments. Keeping r+ from smaug.

Comment 4

[Guilherme Gonçalves [:ggp]]

https://tbpl.mozilla.org/?tree=Try&rev=f39af0aff506

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/da5456a94c1b

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[:aceman]

It seems this breaks compilation with --disable-webspeech ... Can you please fix it?

Comment 8

[Guilherme Gonçalves [:ggp]]

Attachment: Only build SpeechRecognitionError when MOZ_WEBSPEECH is enabled

Sorry about that. This patch should fix it.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 768102 [details] [diff] [review]
Only build SpeechRecognitionError when MOZ_WEBSPEECH is enabled

I guess that works (I'm not familiar with the syntax moz.build uses)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f1e1dbd0b6e

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0f1e1dbd0b6e

Comment 12

[:aceman]

Seems to work, thanks.