Implement SpeechRecognition::interimResults

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kdavis, Assigned: kdavis)

Tracking

unspecified
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

(Whiteboard: [webspeechapi][vaani][systemsfe])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
Currently SpeechRecognition::interimResults is not implemented
(Assignee)

Updated

4 years ago
Assignee: nobody → kdavis
Whiteboard: [webspeechapi][vaani][systemsfe]
(Assignee)

Updated

4 years ago
Depends on: 1185018
(Assignee)

Comment 1

4 years ago
Part 1 of 1: Implemented SpeechRecognition::interimResults                      
                                                                                
Part 1 of 1 for this bug.                                                       
                                                                                
This patch implements SpeechRecognition::interimResults described in the Web    
Speech API specification                                                        
                                                                                
https://dvcs.w3.org/hg/speech-api/raw-file/tip/webspeechapi.html#dfn-interimresults
                                                                                
This implementation is particularly simple as the Web Speech API requires of    
the attribute SpeechRecognition::interimResults that                            
                                                                                
1. Default value MUST be false                                                  
2. If true, interim results SHOULD be returned                                  
3. If false, interim results MUST NOT be returned                               
                                                                                
As Pocketsphinx does not return interm results, we can trivially implement this 
attribute in a conforming manner by                                             
                                                                                
1. Defaulting SpeechRecognition::mInterimResults to false                       
2. Ignoring the subsequent value of mInterimResults                             
                                                                                
The try for this patch is here https://treeherder.mozilla.org/#/jobs?repo=try&revision=4a7c4b790e36
Attachment #8636545 - Flags: review?(bugs)
(Assignee)

Updated

4 years ago
Blocks: 1185235
Comment on attachment 8636545 [details] [diff] [review]
Part 1 of 1: Implemented SpeechRecognition::interimResults

Again, webidl shouldn't need [Throws] anymore, and ErrorResult& aRv could be dropped.
that is so trivial change that perhaps just do it in this bug and not in a followup.
But either way.
Attachment #8636545 - Flags: review?(bugs) → review+
(Assignee)

Comment 3

4 years ago
(In reply to Olli Pettay [:smaug] from comment #2)
> Comment on attachment 8636545 [details] [diff] [review]
> Part 1 of 1: Implemented SpeechRecognition::interimResults
> 
> Again, webidl shouldn't need [Throws] anymore, and ErrorResult& aRv could be
> dropped.
> that is so trivial change that perhaps just do it in this bug and not in a
> followup.
> But either way.

Ok I'll do it in this bug.
(Assignee)

Comment 4

4 years ago
Part 1 of 1: Implemented SpeechRecognition::interimResults

Part 1 of 1 for this bug.

This has no functional changes from the previous patch. It
only makes the change suggested in Comment 2, removing the
throws specifier from the webidl attribute interimResults
on SpeechRecognition.

The try for this patch is here https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b673a6fc726

The interdiff between this patch and the last patch results only
from the change suggested in Comment 2 and is as follows:

diff -u b/dom/media/webspeech/recognition/SpeechRecognition.cpp b/dom/media/webspeech/recognition/SpeechRecognition.cpp
--- b/dom/media/webspeech/recognition/SpeechRecognition.cpp
+++ b/dom/media/webspeech/recognition/SpeechRecognition.cpp
@@ -670,13 +670,13 @@
 }
 
 bool
-SpeechRecognition::GetInterimResults(ErrorResult& aRv) const
+SpeechRecognition::InterimResults() const
 {
   return mInterimResults;
 }
 
 void
-SpeechRecognition::SetInterimResults(bool aArg, ErrorResult& aRv)
+SpeechRecognition::SetInterimResults(bool aArg)
 {
   mInterimResults = aArg;
   return;
diff -u b/dom/media/webspeech/recognition/SpeechRecognition.h b/dom/media/webspeech/recognition/SpeechRecognition.h
--- b/dom/media/webspeech/recognition/SpeechRecognition.h
+++ b/dom/media/webspeech/recognition/SpeechRecognition.h
@@ -82,9 +82,9 @@
 
   void SetContinuous(bool aArg, ErrorResult& aRv);
 
-  bool GetInterimResults(ErrorResult& aRv) const;
+  bool InterimResults() const;
 
-  void SetInterimResults(bool aArg, ErrorResult& aRv);
+  void SetInterimResults(bool aArg);
 
   uint32_t GetMaxAlternatives(ErrorResult& aRv) const;
 
only in patch2:
unchanged:
--- a/dom/webidl/SpeechRecognition.webidl
+++ b/dom/webidl/SpeechRecognition.webidl
@@ -16,17 +16,16 @@
 interface SpeechRecognition : EventTarget {
     // recognition parameters
     [Throws]
     attribute SpeechGrammarList grammars;
     [Throws]
     attribute DOMString lang;
     [Throws]
     attribute boolean continuous;
-    [Throws]
     attribute boolean interimResults;
     [Throws]
     attribute unsigned long maxAlternatives;
     [Throws]
     attribute DOMString serviceURI;
 
     // methods to drive the speech interaction
     [Throws, UnsafeInPrerendering]
Attachment #8636545 - Attachment is obsolete: true
Attachment #8639756 - Flags: review+
(Assignee)

Comment 5

4 years ago
Part 1 of 1: Implemented SpeechRecognition::interimResults

Part 1 of 1 for this bug.

This patch makes no functional changes relative to the patch it replaces.

However, on my machine git had problems merging the previous patch in light
of the recent changes of bug 1185018 comment 17.

So this patch make sure the merge works correctly.

The try for this patch is here https://treeherder.mozilla.org/#/jobs?repo=try&revision=316c0986872a

The interdiff between this patch and the one it replaces is empty.
Attachment #8639756 - Attachment is obsolete: true
Attachment #8642254 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/10bdc996eb3c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.