Skip to content

Clarify epub:type use and restrictions on svgs#2574

Merged
mattgarrish merged 7 commits intomainfrom
fix/issue-2555
Aug 9, 2023
Merged

Clarify epub:type use and restrictions on svgs#2574
mattgarrish merged 7 commits intomainfrom
fix/issue-2555

Conversation

@mattgarrish
Copy link
Member

@mattgarrish mattgarrish commented Jul 21, 2023

This largely implements the text in #2555 (comment) but I tried to further clean up the section to make it more readable.

The big difference is I split the embedding definitions out into a list and included the requirements for each in the bullets. The definitions were hard to read in a single paragraph and having the requirements follow them made the section sound jumpy.

It also makes the fix in issue #2555 to allow the epub:type attribute on renderable elements.

Fixes #2555
Fixes #2556


Preview | Diff

@mattgarrish
Copy link
Member Author

I was looking at the rest of the spec and found a couple of places where we referred to "embedded SVGs" generally when we meant included SVGs.

I also noticed that the svg content documents section requires standalone svg file conformance, so that conflicted with the embedding section referring to svg document fragments for both embedding types. I moved the fragment reference into the inclusion definition.

@iherman iherman added the Change-Class-2 Requested changes are of class 2 (per process) label Jul 22, 2023
@iherman
Copy link
Member

iherman commented Jul 22, 2023

+1 on the content change.

Process question to @plehegar. This is a class-2 change. My understanding is that such change can be made on the document without further ado (publishing the result through the webmaster, that is). However, if this version is combined with another, class-3 change (#2575 in this case), does my remark in #2575 (comment) concern the class-2 changes as well, i.e., they should also be marked via ins/del elements, or is it o.k. to do direct change on the document for these?

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 change looks OK to me.

About the specific issue of how the epub:type is defined for included SVG, I still find the reading a little mazy:

  • the "by inclusion" bullet says content conformance constraints are defined by 6.2.3… but not only, wait for the next paragraph.
  • we need to make a special case for epub:type, by pointing to a bullet in the restrictions for SVG content documents.

What about moving instead the epub:type SVG restrictions from the bullet in "6.2.2 SVG Requirements" to a new paragraph in "6.2.3 Restrictions on SVG"?

To summarize, what I'm suggesting is:

  • in 6.2.2:

replace

MAY include the epub:type attribute for expressing structural semantics. When specified, the attribute MUST only be included on structural, shape, or text elements [svg].

by

MAY include the epub:type attribute for expressing structural semantics. When specified, the attribute MUST conform to constraints expressed in 6.2.3 Restrictions on SVG

  • in 6.2.3, add a new bullet in the restrictions list:

the epub:type attribute MUST only be included on structural, shape, or text elements [svg].

  • in 6.1.4.2, the paragraph "Although the XHTML content (…) in the same way as for SVG content documents." can now be removed.

@mattgarrish
Copy link
Member Author

Maybe we can do without mentioning epub:type in 6.2.2. I hate splitting the allowed and restricted parts of the attribute. It's sort of like if we had separate sections in the HTML extensions and constraint subsections to define each part of how you can use epub:type.

SVG already allows foreign namespaced attributes and elements, so we're not actually allowing anything in 6.2.2 that the SVG spec doesn't already allow. The two "MAY" bullets could be expressed as a note without changing anything normative.

I'm going to play around with this a bit more.

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.

Looks cleaner and more readable to me! 👍

@mattgarrish
Copy link
Member Author

@iherman I'm wondering if we should merge this with #2575?

I'm not sure it qualifies as a class 3 change, as we're only taking a couple of unnecessary MAY statements and turning them into a note, but it's going to make a merge conflict because the bullet that 2575 is changing is now in a different section. It might be simpler to make all these epub:type in svg changes together.

@mattgarrish mattgarrish changed the title Clarify epub:type restrictions on included svgs Clarify epub:type use and restrictions on svgs Jul 27, 2023
@mattgarrish
Copy link
Member Author

I've updated this pull request to incorporate the changes that were in #2575.

@mattgarrish mattgarrish added Change-Class-3 Requested changes are of class 3 (per process) and removed Change-Class-2 Requested changes are of class 2 (per process) labels Aug 8, 2023
@mattgarrish mattgarrish mentioned this pull request Aug 8, 2023
@mattgarrish mattgarrish merged commit 8478066 into main Aug 9, 2023
@mattgarrish mattgarrish deleted the fix/issue-2555 branch August 9, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Change-Class-3 Requested changes are of class 3 (per process)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restriction on epub:type in svg is too strict Clarify epub:type use with embedded SVG

3 participants