-
Notifications
You must be signed in to change notification settings - Fork 29
Introduce a generic skip action as a replacement to skipad. #350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 8 commits
7492ecb
61e4594
5fb2ce3
21fbe0c
986cf70
652be79
fdab4e7
d7e2f94
bc4d50c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -383,8 +383,8 @@ platform UI or media keys, thereby improving the user experience. | |
| the playback has a notion of playlist. | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaSessionAction>skipad</dfn>: the action's | ||
| intent is to skip the advertisement that is currently playing. | ||
| <dfn enum-value for=MediaSessionAction>skip</dfn>: the action's | ||
| intent is to skip the media item that is currently playing. | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaSessionAction>stop</dfn>: the action's intent | ||
|
|
@@ -751,7 +751,7 @@ enum MediaSessionAction { | |
| "seekforward", | ||
| "previoustrack", | ||
| "nexttrack", | ||
| "skipad", | ||
| "skip", | ||
| "stop", | ||
| "seekto", | ||
| "togglemicrophone", | ||
|
|
@@ -1079,6 +1079,7 @@ interface MediaMetadata { | |
| attribute DOMString title; | ||
| attribute DOMString artist; | ||
| attribute DOMString album; | ||
| readonly attribute MediaKind kind; | ||
| attribute FrozenArray<object> artwork; | ||
| [SameObject] readonly attribute FrozenArray<ChapterInformation> chapterInfo; | ||
| }; | ||
|
|
@@ -1087,9 +1088,21 @@ dictionary MediaMetadataInit { | |
| DOMString title = ""; | ||
| DOMString artist = ""; | ||
| DOMString album = ""; | ||
| MediaKind kind = ""; | ||
| sequence<MediaImage> artwork = []; | ||
| sequence<ChapterInformationInit> chapterInfo = []; | ||
| }; | ||
|
|
||
| enum MediaKind { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a section describing what these mean? "advertisement" is obvious to me, but the rest are not so obvious
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I updated the enum and added some description.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the implementation make use of these values, e.g., in a default action handler? I'm wondering if an enum is needed and we could let the web app set any value it wants, using Also, a web app might want to skip between chapters, so do we need similar on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right, this PR adds
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, thanks.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This list may be a bit too specific, for example BBC might want to offer a skip to next news item, skip to next song, or whatever. |
||
| "advertisement", | ||
| "closing-credits", | ||
| "cold-open-scene", | ||
| "opening-credits", | ||
| "post-credits-scene", | ||
| "summary", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of summary, recap might be a better word? It matches current impl via content providers and recap more closely matches this element's description. |
||
| "content", | ||
| "" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe let's just call this "content" ? or "other".
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed it to |
||
| }; | ||
| </pre> | ||
|
|
||
| <p> | ||
|
|
@@ -1109,6 +1122,46 @@ dictionary MediaMetadataInit { | |
| which are DOMString. | ||
| </p> | ||
|
|
||
| <p> | ||
| A {{MediaMetadata}} has an associated <dfn for="MediaMetadata">kind</dfn>, | ||
| which is a {{MediaKind}}, which can have one of the following value: | ||
|
youennf marked this conversation as resolved.
|
||
| </p> | ||
| <ul> | ||
| <li> | ||
| <dfn enum-value for=MediaKind>advertisement</dfn>: the media content represents advertisments. | ||
|
chrisn marked this conversation as resolved.
Outdated
|
||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaKind>closing-credits</dfn>: the media content represents closing credits, | ||
| typically happening at the end of a video. | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaKind>cold-open-scene</dfn>: the media content represents a cold open scene, | ||
| typically happening at the very beginning of a video, before opening credits. | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaKind>opening-credits</dfn>: the media content represents opening credits, | ||
| typically happening at the beginning of a video. | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaKind>post-credits-scene</dfn>: the media content represents a post credits scene, | ||
| typically happening after the closing credits. | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaKind>summary</dfn>: the media content represents a summary, | ||
| typically happening at the beginning of a TV series episode and summarizing past episodes. | ||
| </li> | ||
| <li> | ||
| <dfn enum-value for=MediaKind>content</dfn>: the media content represents the main media content, | ||
| like a song or a TV series episode.. | ||
|
chrisn marked this conversation as resolved.
Outdated
|
||
| </li> | ||
| <li> | ||
| the empty string: the default value of media content, it represents any other value of a part of media content. | ||
| </li> | ||
| </ul> | ||
| <p class=note> | ||
| This enumeration is experimental and is subject to changes. | ||
| </p> | ||
|
youennf marked this conversation as resolved.
|
||
|
|
||
| <p> | ||
| A {{MediaMetadata}} has an associated sequence of <dfn | ||
| for="MediaMetadata">artwork images</dfn>, which is a sequence of type | ||
|
|
@@ -1130,6 +1183,7 @@ dictionary MediaMetadataInit { | |
| <li>Its <a for=MediaMetadata>title</a> is the empty string.</li> | ||
| <li>Its <a for=MediaMetadata>artist</a> is the empty string.</li> | ||
| <li>Its <a for=MediaMetadata>album</a> is the empty string.</li> | ||
| <li>Its <a for=MediaMetadata>kind</a> is the empty string.</li> | ||
| <li>Its <a for=MediaMetadata title='artwork image'>artwork images</a> length | ||
| is <code>0</code>.</li> | ||
| <li>Its <a for=MediaMetadata>chapter information</a> length is | ||
|
|
@@ -1156,6 +1210,10 @@ dictionary MediaMetadataInit { | |
| Set <var>metadata</var>'s {{MediaMetadata/album}} to | ||
| <var>init</var>'s {{MediaMetadataInit/album}}. | ||
| </li> | ||
| <li> | ||
| Set <var>metadata</var>'s {{MediaMetadata/kind}} to | ||
| <var>init</var>'s {{MediaMetadataInit/kind}}. | ||
| </li> | ||
| <li> | ||
| Run the <a>convert artwork algorithm</a> with <var>init</var>'s | ||
| {{MediaMetadataInit/artwork}} as <var>input</var> and set | ||
|
|
@@ -1245,6 +1303,11 @@ user agent MUST run the following steps: | |
| set the {{MediaMetadata}}'s <a for=MediaMetadata>album</a> to the given value. | ||
| </p> | ||
|
|
||
| <p> | ||
| The <dfn attribute for="MediaMetadata">kind</dfn> attribute reflects the | ||
| {{MediaMetadata}}'s <a for=MediaMetadata>kind</a>. On getting, it MUST return | ||
| the {{MediaMetadata}}'s <a for=MediaMetadata>kind</a>.</p> | ||
|
|
||
| <p> | ||
| The <dfn attribute for="MediaMetadata">artwork</dfn> | ||
| attribute reflects the {{MediaMetadata}}'s <a for="MediaMetadata">artwork | ||
|
|
@@ -1344,12 +1407,14 @@ interface</h2> | |
| interface ChapterInformation { | ||
| readonly attribute DOMString title; | ||
| readonly attribute double startTime; | ||
| readonly attribute MediaKind kind; | ||
|
||
| [SameObject] readonly attribute FrozenArray<MediaImage> artwork; | ||
| }; | ||
|
|
||
| dictionary ChapterInformationInit { | ||
| DOMString title = ""; | ||
| double startTime = 0; | ||
| MediaKind kind = ""; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question here about whether it should be "other" |
||
| sequence<MediaImage> artwork = []; | ||
| }; | ||
|
|
||
|
|
@@ -1373,6 +1438,12 @@ dictionary ChapterInformationInit { | |
| startTime</dfn> which is double. | ||
| </p> | ||
|
|
||
| <p> | ||
| A {{ChapterInformation}} has an associated <dfn | ||
| for="ChapterInformation">kind</dfn> | ||
| which is a {{MediaKind}}. | ||
| </p> | ||
|
|
||
| <p> | ||
| A {{ChapterInformation}} has an associated list of <dfn | ||
| for="ChapterInformation"> | ||
|
|
@@ -1397,6 +1468,10 @@ dictionary ChapterInformationInit { | |
| for=ChapterInformation>startTime</a> is negative or greater than | ||
| [=duration=], throw a <a exception>TypeError</a>. | ||
| </li> | ||
| <li> | ||
| Set <var>chapterInfo</var>'s {{ChapterInformation/kind}} to | ||
| <var>init</var>'s {{ChapterInformationInit/kind}}. | ||
| </li> | ||
| <li> | ||
| Let {{ChapterInformationInit/artwork}} be the result of running the | ||
| <a>convert artwork algorithm</a> with <var>init</var>'s | ||
|
|
@@ -1425,6 +1500,12 @@ dictionary ChapterInformationInit { | |
| for=ChapterInformation>startTime</a>. | ||
| </p> | ||
|
|
||
| <p> | ||
| The <dfn attribute for="ChapterInformation">kind</dfn> attribute reflects the | ||
| {{ChapterInformation}}'s <a for=ChapterInformation>kind</a>. On getting, it | ||
| MUST return the {{ChapterInformation}}'s <a for=ChapterInformation>kind</a>. | ||
| </p> | ||
|
|
||
| <p> | ||
| The <dfn attribute for="ChapterInformation">artwork</dfn> | ||
| attribute reflects the {{ChapterInformation}}'s <a | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'kind' attribute is marked as readonly in the interface but can be set through MediaMetadataInit. This inconsistency could confuse API consumers about whether the property is mutable after construction.