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.

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

update core media type requirements for picture and img #1247

Merged
merged 2 commits into from
Mar 21, 2019

Conversation

mattgarrish
Copy link
Member

This PR fixes #1237 (again) as described in #1237 (comment):

  • when inside a picture the only time foreign resources can be referenced is from a source element that explicitly identifies that it references a foreign media type in its type attribute
  • otherwise, the img element requires a core media type fallback whenever its src or srcset attributes reference foreign resources.

@rdeltour please verify that I have this right, and that the spec prose reflects your proposal.

@mattgarrish mattgarrish requested review from rdeltour and dauwhe March 14, 2019 19:31
@mattgarrish
Copy link
Member Author

Copy link
Member

@rdeltour rdeltour left a comment

Choose a reason for hiding this comment

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

The changes do reflect my proposal accurately 👍 (I hope we got it right this time 😅).

I made a couple nitpicking comments inline, I'll let you decide whether they're worth an update or not.

<ul>
<li>it MUST reference Core Media Type Resources from its <code>src</code> and
<code>srcset</code> attributes, when those attributes are specified;</li>
<li>any child <a
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: "any child source elements" might need to explicit the parent context here, as the main subject is the img element.
We could say "any child source elements of this picture element" (referencing the wording introducing this list item) or "any sibling source elements of the img element".

Note btw that only the preceding source siblings will be used to define the img source set. But we may not need to go to this level of detail…

Copy link
Member Author

Choose a reason for hiding this comment

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

Blah, right, "child" was from an earlier drafting. I'll change it to sibling.

href="https://www.w3.org/TR/html/semantics-embedded-content.html#the-source-element"
><code>source</code> elements</a> also MUST reference Core Media Type
Resources from their <code>src</code> and <code>srcset</code> attributes unless the
element specifies that it references a Foreign Resource media type in its
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: "specifies that it references a Foreign Resource media type in its type attribute" is understandable but might sound a little odd or hard to parse?

  • we never explicitly specify what is a Foreign Resource media type (is it any media type that is not a CMT? or the media type of a Foreign Resource that is correctly declared in the Package Doc?).
  • also, a source element can't be said to "reference a (…) media type"
  • it doesn't really take into account MIME type parameters, which are allowed in the type attribute value.

In HTML/WHATWG lingo, we could say something along the lines of:

unless its type specifies a MIME type whose essence is listed as a Core Media Type.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we use "essence", everyone has to take a trip to another specification... :)

How about we just keep this in epub parlance by restating it as:

unless it specifies a media type in its type attribute that is not a Core Media Type.

Copy link
Member

Choose a reason for hiding this comment

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

How about we just keep this in epub parlance by restating it as:

unless it specifies a media type in its type attribute that is not a Core Media Type.

yes, works for me!

If we use "essence", everyone has to take a trip to another specification... :)

meh, where's the fun when not doing that? 😁🤓

</li>
<li>Otherwise, it MAY reference Foreign Resources in its <code>src</code> and
<code>srcset</code> attributes provided a <a
href="epub-packages.html#sec-foreign-restrictions-manifest">manifest fallback</a>
Copy link
Member

Choose a reason for hiding this comment

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

s/manifest fallback/manifest fallbacks/ ?

@dauwhe dauwhe merged commit 81e585d into master Mar 21, 2019
@dauwhe dauwhe deleted the fix/issue-1237/revised-cmt branch March 21, 2019 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fallback requirements for picture element
3 participants