Skip to content
This repository has been archived by the owner on Feb 26, 2021. It is now read-only.

"Play Spotify" command plays "ヒプノシスRADIO -Spotify Edition-" podcast #471

Closed
alexandra-martin opened this issue Oct 23, 2019 · 3 comments
Assignees
Labels
bug Something isn't working Verified Fix was validated by QA
Milestone

Comments

@alexandra-martin
Copy link

alexandra-martin commented Oct 23, 2019

Prerequisites:

Mic permissions are enabled.
A Spotify tab is made the active tab and the music is paused.

STR:

  1. Use the shortcut or click on the mic icon from the browser toolbar.
  2. Say or write the "Play Spotify" command.

Expected result:

The Spotify music is playing again.

Actual result:

"ヒプノシスRADIO -Spotify Edition-" podcast is searched in the tab and starts playing.

Notes:

Reproduced on Mac 10.14.6 and Win10x64 with Firefox Nightly 72.0a1 (64-bit).
The "Play music" command works as intended.

5

@alexandra-martin alexandra-martin added the bug Something isn't working label Oct 23, 2019
@ianb
Copy link
Contributor

ianb commented Oct 23, 2019

This is because of a conflict between two intents, the play intent: play [query] and the unpause intent: (unpause | continue | play) [service:musicServiceName]

The prioritization heuristic doesn't distinguish between the generic [query] slot and the more specific [server:musicServiceName] intent.

To fix this we'll need to keep track of slot types when compiling expressions, and then rank matches using this information (probably counting any typed slot as 1 character – reasonable, since it is on the order of one character of information (though a fancier option would score types based on how limited their matches were, maybe using floating point numbers).

@ianb
Copy link
Contributor

ianb commented Oct 23, 2019

I'm going to mark this as a good first issue, but it's a hard good first issue. But not too hard!

The work is entirely inside intentParser.js. You'll have to look at these parts:

To keep track of the slot types, put something here to store slot types in a property on the Matcher object:

https://github.com/mozilla/firefox-voice/blob/1d26885d84a32e6cca34987cdf472a9a2517a025/extension/background/intentParser.js#L70-L80

Then you have to return the slot types with the match, similar to parameters:

https://github.com/mozilla/firefox-voice/blob/1d26885d84a32e6cca34987cdf472a9a2517a025/extension/background/intentParser.js#L36-L41

Then you have to make use of that when deciding on the best match:

https://github.com/mozilla/firefox-voice/blob/1d26885d84a32e6cca34987cdf472a9a2517a025/extension/background/intentParser.js#L222-L236

And to test it, I'd just put play spotify into the examples, and use show all intents to make sure it parses correctly.

@ianb ianb added good first issue Good for newcomers and removed good first issue Good for newcomers labels Oct 23, 2019
@ianb ianb self-assigned this Oct 25, 2019
@ianb ianb modified the milestones: Backlog, Code Quality Oct 25, 2019
@ianb ianb closed this as completed in 65eacdb Oct 25, 2019
ianb added a commit that referenced this issue Oct 25, 2019
Fix #471, prioritize intent matches that have typed slots
@alexandra-martin
Copy link
Author

Verified on Mac 10.14.6 and Win10x64 with Firefox Nightly 72.0a1 (64-bit).

1

@alexandra-martin alexandra-martin added the Verified Fix was validated by QA label Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working Verified Fix was validated by QA
Projects
None yet
Development

No branches or pull requests

2 participants