Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Manifest-icon-like metadata artworks. #129

Merged
merged 4 commits into from
Jul 4, 2016
Merged

Manifest-icon-like metadata artworks. #129

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f007af
Manifest-icon-like metadata artworks.
May 31, 2016
ab208be
addressed nits
Jun 2, 2016
16a7d79
addressed comments
Jun 6, 2016
f18ff32
MediaArtwork->MediaImage to avoid naming confusions
Jun 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 164 additions & 6 deletions mediasession.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ urlPrefix: http://www.w3.org/TR/page-visibility/; spec: PAGE-VISIBILITY
type: enum; urlPrefix: #pv-page-
text: visible
text: hidden
urlPrefix: http://www.w3.org/TR/appmanifest/; spec: appmanifest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to find good URL, filed w3c/manifest#469 instead.

In the meantime, can you use https here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

type: dfn
text: image object; url: #dfn-image-object
urlPrefix: https://heycam.github.io/webidl/
type: exception
text: InvalidStateError
Expand Down Expand Up @@ -289,8 +292,18 @@ attribute must return the <a>media session</a>'s <a>kind</a>.

The <dfn attribute for="MediaSession"><code>metadata</code></dfn> attribute, on
getting, must return the <a>media session</a>'s <a lt="media session
metadata">metadata</a>. On setting, the <a>media session</a>'s <a lt="media
session metadata">metadata</a> must be set to the new value.
metadata">metadata</a>. On setting, the user agent must run the following steps:

<ol>
<li>
Set the <a>media session</a>'s <a lt="media session metadata">metadata</a>
to the new value.
</li>
<li>
Run the <a>fetch steps</a> for <a>media session</a>'s <a lt="media session
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked a lot about this with @doomdavve and it is the topic of #58

My conclusion from all of that was that we should probably "keep the 'fetch steps' as late as possible, and lean on preloading to handle this" and I did that in b2deaa2

What's the intention for https://codereview.chromium.org/2013813002?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Rephrased these words, and moved this part to the "MediaMetadata" section (which seems to be a better place)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the intention for https://codereview.chromium.org/2013813002?

The CL implements this spec in Blink, and will be updated when we decided the spec details.

metadata">metadata</a>.
</li>
</ol>

The <dfn method for="MediaSession"><code>activate()</code></dfn> method, when
invoked, must run these steps:
Expand Down Expand Up @@ -1123,15 +1136,21 @@ interface MediaMetadata {
readonly attribute DOMString title;
readonly attribute DOMString artist;
readonly attribute DOMString album;
readonly attribute sequence&lt;MediaArtwork> artworks;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use FrozenArray here while things are readonly (I don't think it should be readonly but that's orthogonal ;)).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

};

dictionary MediaMetadataInit {
DOMString title = "";
DOMString artist = "";
DOMString album = "";
sequence&lt;MediaArtworkInit>? artworks;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what's the right type here. I would say that an simple array would work fine because this is basically meant to be write-only. @annevk WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm keeping sequence for now. Changed this to:
sequence artworks = [];

Will update this when @annevk make comments

};
</pre>

A {{MediaMetadata}} object has a <dfn for="MediaMetadata">title</dfn>, an <dfn
for="MediaMetadata">artist</dfn>, an <dfn for="MediaMetadata">album</dfn> and a
sequence of <dfn for="MediaMetadata">artwork</dfn>s.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can link to artwork while using the plural form with title:

<dfn for="MediaMetadata" title="artwork">artworks</dfn>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


The <dfn constructor
for="MediaMetadata"><code>MediaMetadata(<var>init</var>)</code></dfn>
constructor, when invoked, must run the following steps:
Expand All @@ -1141,16 +1160,149 @@ constructor, when invoked, must run the following steps:
Let <var>metadata</var> be a new {{MediaMetadata}} object.
</li>
<li>
Set <var>metadata</var>'s {{MediaMetadata/title}}, {{MediaMetadata/artist}},
and {{MediaMetadata/album}} attributes to the values of <var>init</var>'s
{{MediaMetadataInit/title}}, {{MediaMetadataInit/artist}}, and
{{MediaMetadataInit/album}} members, respectively.
Set <var>metadata</var>'s {{MediaMetadata/title}} to <var>init</var>'s
{{MediaMetadataInit/title}}.
</li>
<li>
Set <var>metadata</var>'s {{MediaMetadata/artist}} to <var>init</var>'s
{{MediaMetadataInit/artist}}.
</li>
<li>
Set <var>metadata</var>'s {{MediaMetadata/album}} to
<var>init</var>'s {{MediaMetadataInit/album}}.
</li>
<li>
Set <var>metadata</var>'s {{MediaMetadata/artworks}} using <var>init</var>'s
{{MediaMetadataInit/artworks}} by calling the
<a><code>MediaArtwork(init)</code></a> constructor.
</li>
<li>
Return <var>metadata</var>.
</li>
</ol>

The <dfn attribute for="MediaMetadata"><code>title</code></dfn> attribute must
return the {{MediaMetadata}} objects's <a>title</a>.

The <dfn attribute for="MediaMetadata"><code>artist</code></dfn> attribute must
return the {{MediaMetadata}} objects's <a>artist</a>.

The <dfn attribute for="MediaMetadata"><code>album</code></dfn> attribute must
return the {{MediaMetadata}} objects's <a>album</a>.

The <dfn attribute for="MediaMetadata"><code>artworks</code></dfn> attribute
must return the {{MediaMetadata}} objects's <a>artwork</a>s, as a sequence of
{{MediaArtwork}}s. The <a attribute for="MediaMetadata">artworks</a> attribute
can be empty.

The <dfn>fetch steps</dfn> for a given {{MediaMetadata}} object
<var>metadata</var> are:

<ol>
<!-- XXX https://www.w3.org/Bugs/Public/show_bug.cgi?id=24055 -->
<li>
If <var>metadata</var>'s <a><code>artworks</code></a> is empty, then
terminate these steps.
</li>
<li>
If the platform supports displaying media artwork, select a <dfn>prefered
artwork</dfn>
from <var>metadata</var>'s <a><code>artworks</code></a>.
</li>
<li>
<a lt="fetch">Fetch</a> <a>prefered artwork</a>'s {{MediaArtwork/src}}.

Then, <a>in parallel</a>:

<ol>
<li>
Wait for the <a>response</a>.
</li>
<li>
If the <a>response</a>'s <a>internal response</a>'s
<a lt="response type">type</a> is <i>default</i>, attempt to decode the
resource as image.
</li>
<li>
If the image format is supported, use the image as the artwork for
display. (Otherwise the user agent can select another <a>prefered
artwork</a> and try again.)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's common practice to try again with another source when an image resource fails to decode or turns out to be unsupported. Is there precedent for this? In notifications, we just fall back to having nothing or a default resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we say "the user agent may have fallback behavior such as displaying an default image as artwork."

</li>
</ol>
</li>
</ol>

<h2 id="the-mediaartwork-interface">The {{MediaArtwork}} interface</h2>

<pre class="idl">

[Constructor(MediaArtworkInit init)]
interface MediaArtwork {
readonly attribute DOMString src;
readonly attribute DOMString sizes;
readonly attribute DOMString type;
};

dictionary MediaArtworkInit {
DOMString src = "";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hot new thing is to use USVString for strings that are urls.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

DOMString sizes = "";
DOMString type = "";
};
</pre>

A {{MediaArtwork}} object has a <dfn for="MediaArtwork">source</dfn>, a list of
<dfn for="MediaArtwork">sizes</dfn>, and a <dfn for="MediaArtwork">type</dfn>.

The <dfn constructor
for="MediaArtwork"><code>MediaMetadata(<var>init</var>)</code></dfn>
constructor, when invoked, must run the following steps:

<ol>
<li>
Let <var>metadata</var> be a new {{MediaArtwork}} object.
</li>
<li>
Set <var>metadata</var>'s {{MediaArtwork/src}} to <var>init</var>'s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to resolve the URL at some point, and I think doing it here (or in the MediaMetadata constructor if the MediaArtwork interface goes away) makes for the most sane behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now done in the MediaArtwork(init) constructor.
This may change if we decide not to use MediaArtwork+MediaArtworkInit pattern

{{MediaArtworkInit/src}}.
</li>
<li>
Set <var>metadata</var>'s {{MediaArtwork/sizes}} to <var>init</var>'s
{{MediaArtworkInit/sizes}}.
</li>
<li>
Set <var>metadata</var>'s {{MediaArtwork/type}} to <var>init</var>'s
{{MediaArtworkInit/type}}.
</li>
<li>
Return <var>metadata</var>.
</li>
</ol>


The MediaArtwork <a attribute for="MediaArtwork">src</a>, <a attribute
for="MediaArtwork">sizes</a> and <a attribute for="MediaArtwork">type</a>
conforms to the <a>image object</a>s in Web App Manifest.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't say that it "conforms to" but that it is "inspired from".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


The <dfn attribute for="MediaArtwork">src</dfn> attribute must return the
{{MediaArtwork}} object's <a for="MediaArtwork">source</a>. It is a URL from
which the user agent can fetch the image's data.

The <dfn attribute for="MediaArtwork">sizes</dfn> attribute must return the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like https://www.w3.org/TR/appmanifest/#sizes-member needs to be separated out into a shared definition and used here.

Also, is there really no definition of the parsing of this string? Does it really fail fatally in implementations if there's a leading zero, extra spaces, etc.?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sizes and its parsing rules are defined in HTML, manifest basically defers to this: https://html.spec.whatwg.org/#attr-link-sizes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected the link. Now it refers to the HTMLLinkElement sizes attribute.

{{MediaArtwork}} object's <a for="MediaArtwork">sizes</a>. It is a string
consisting of an unordered set of unique space-separated tokens which are AScII
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/AScII/ASCII/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

case insensitive that represents the dimensions of an image. Each keyword is
either an ASCII case-insensitive match for the string "any", or a value that
consists of two valid non-negative integers that do not have a leading U+0030
DIGIT ZERO (0) character and that are separated by a single U+0078 LATIN SMALL
LETTER X or U+0058 LATIN CAPITAL LETTER X character. The keywords represent icon
sizes in raw pixels (as opposed to CSS pixels). When multiple image objects are
available, a user agent may use the value to decide which icon is most suitable
for a display context (and ignore any that are inappropriate).

The <dfn attribute for="MediaArtwork">type</dfn> attribute must return the
{{MediaArtwork}} object's <a for="MediaArtwork">type</a>. It is a MIME type for
deciding the media type of the image.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"for deciding the media type" doesn't sound correct. Maybe you could re-use some phrasing from the Manifest spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


<h2 id="extensions-to-the-htmlmediaelement-interface">Extensions to the
{{HTMLMediaElement}} interface</h2>

Expand Down Expand Up @@ -1353,6 +1505,9 @@ When the user agent is to <dfn>resume a web audio object</dfn> for a given
title: "Episode Title",
artist: "Podcast Host",
album: "Podcast Title",
artworks: {
src:"podcast.jpg"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't sound correct, it's not an array :) Also, maybe you could have an example using the other fields of the object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and added new examples for multiple artworks.

}
});
</pre>
</div>
Expand Down Expand Up @@ -1386,6 +1541,9 @@ When the user agent is to <dfn>resume a web audio object</dfn> for a given
title: event.target == audio1 ? "Chapter 1" : "Chapter 2",
artist: "An Author",
album: "A Book",
artworks: {
src: "cover.jpg"
}
});
}

Expand Down
Loading