Skip to content

Commit 2091d14

Browse files
rdeltourrdeltour
committed
feat: check fragment requirements on overlays links to text content
This commit checks the updated requirements on referencing document fragments from media overlays. - Warning `MED-017` (new) is reported for fragment references to XHTML content documents that do not indicate an element ID - Warning `MED-018` (new) is reported for fragment references SVG content document that are not valid SVG fragment identifiers Fixes #1248, fixes #1301
1 parent e4a69a5 commit 2091d14

File tree

44 files changed

+285
-114
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+285
-114
lines changed

‎src/main/java/com/adobe/epubcheck/dtbook/DTBookHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public void startElement()
5151
if (ns.equals("http://www.daisy.org/z3986/2005/dtbook/"))
5252
{
5353
// Register IDs
54-
xrefChecker.registerID(e.getAttribute("id"), XRefChecker.Type.HYPERLINK, location());
54+
xrefChecker.registerID(e.getAttribute("id"), XRefChecker.Type.GENERIC, location());
5555

5656
// Check cross-references (link@href | a@href | img@src)
5757
URL url = null;

‎src/main/java/com/adobe/epubcheck/messages/DefaultSeverities.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ private void initialize()
160160
severities.put(MessageId.MED_014, Severity.ERROR);
161161
severities.put(MessageId.MED_015, Severity.USAGE);
162162
severities.put(MessageId.MED_016, Severity.WARNING);
163+
severities.put(MessageId.MED_017, Severity.WARNING);
164+
severities.put(MessageId.MED_018, Severity.WARNING);
163165

164166
// NAV
165167
severities.put(MessageId.NAV_001, Severity.ERROR);

‎src/main/java/com/adobe/epubcheck/messages/MessageId.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ public enum MessageId implements Comparable<MessageId>
154154
MED_014("MED_014"),
155155
MED_015("MED_015"),
156156
MED_016("MED_016"),
157+
MED_017("MED_017"),
158+
MED_018("MED_018"),
157159

158160
// Epub3 based table of content errors
159161
NAV_001("NAV-001"),

‎src/main/java/com/adobe/epubcheck/opf/XRefChecker.java‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ public void checkReferences()
407407
break;
408408
case OVERLAY_TEXT_LINK:
409409
overlayLinks.add(reference);
410+
checkReference(reference);
410411
break;
411412
default:
412413
checkReference(reference);
@@ -419,11 +420,7 @@ public void checkReferences()
419420

420421
private void checkReference(URLReference reference)
421422
{
422-
Resource hostResource = resources.get(reference.location.url);
423-
424-
// Retrieve the Resource instance representing the targeted document
425-
// If the resource was not declared in the manifest,
426-
// we build a new Resource object for the data URL.
423+
// Retrieve the target resource
427424
Resource targetResource = resources.get(reference.targetDoc);
428425
String targetMimetype = (targetResource != null) ? targetResource.getMimeType() : "";
429426

@@ -487,6 +484,7 @@ else if (!undeclared.contains(reference.targetDoc)
487484
switch (reference.type)
488485
{
489486
case HYPERLINK:
487+
490488
if ("epubcfi".equals(fragment.getScheme()))
491489
{
492490
break; // EPUB CFI is not supported
@@ -536,6 +534,14 @@ else if (reference.type == Type.IMAGE && !targetResource.hasImageFallback())
536534
}
537535
}
538536
break;
537+
case OVERLAY_TEXT_LINK:
538+
if (!OPFChecker.isBlessedItemType(targetMimetype, version))
539+
{
540+
report.message(MessageId.RSC_010,
541+
reference.location.context(container.relativize(reference.url)));
542+
return;
543+
}
544+
break;
539545
case SEARCH_KEY:
540546
// TODO update when we support EPUB CFI
541547
if ((!fragment.exists() || !"epubcfi".equals(fragment.getScheme()))
@@ -584,31 +590,27 @@ else if (reference.type == Type.IMAGE && !targetResource.hasImageFallback())
584590
// Fragment integrity checks
585591
if (fragment.exists() && !fragment.isEmpty())
586592
{
587-
// EPUB CFI
588-
if ("epubcfi".equals(fragment.getScheme()))
589-
{
590-
// FIXME HOT should warn if in MO
591-
// FIXME epubcfi currently not supported (see issue 150).
592-
return;
593-
}
594-
// Media fragments in Data Navigation Documents
595-
else if (fragment.isMediaFragment() && hostResource != null && hostResource.hasItem()
596-
&& hostResource.getItem().getProperties()
597-
.contains(PackageVocabs.ITEM_VOCAB.get(PackageVocabs.ITEM_PROPERTIES.DATA_NAV)))
598-
{
599-
// Ignore,
600-
return;
601-
}
602-
// Non-ID-based fragments are ignored
603-
else if (fragment.getId().isEmpty())
593+
// Check media overlays requirements
594+
if (reference.type == Type.OVERLAY_TEXT_LINK)
604595
{
605-
return;
596+
// Check that references to XHTML indicate an element by ID
597+
if (MIMEType.XHTML.is(targetMimetype) && fragment.getId().isEmpty())
598+
{
599+
report.message(MessageId.MED_017, reference.location, fragment.toString());
600+
}
601+
// Check that references to SVG use a SVG fragment identifier
602+
else if (MIMEType.SVG.is(targetMimetype) && !fragment.isValid())
603+
{
604+
report.message(MessageId.MED_018, reference.location, fragment.toString());
605+
}
606606
}
607-
// Fragment Identifier (by default)
608-
else if (!container.isRemote(reference.targetDoc))
607+
608+
// Check ID-based fragments
609+
// Other fragment types (e.g. EPUB CFI) are not currently supported
610+
if (!fragment.getId().isEmpty() && !container.isRemote(reference.targetDoc))
609611
{
610-
ID anchor = targetResource.ids.get(fragment.getId());
611-
if (anchor == null)
612+
ID targetID = targetResource.ids.get(fragment.getId());
613+
if (targetID == null)
612614
{
613615
report.message(MessageId.RSC_012, reference.location.context(reference.url.toString()));
614616
return;
@@ -617,15 +619,16 @@ else if (!container.isRemote(reference.targetDoc))
617619
{
618620
case SVG_PAINT:
619621
case SVG_CLIP_PATH:
620-
if (anchor.type != reference.type)
622+
if (targetID.type != reference.type)
621623
{
622624
report.message(MessageId.RSC_014, reference.location.context(reference.url.toString()));
623625
return;
624626
}
625627
break;
626628
case SVG_SYMBOL:
627629
case HYPERLINK:
628-
if (anchor.type != reference.type && anchor.type != Type.GENERIC)
630+
case OVERLAY_TEXT_LINK:
631+
if (targetID.type != reference.type && targetID.type != Type.GENERIC)
629632
{
630633
report.message(MessageId.RSC_014, reference.location.context(reference.url.toString()));
631634
return;

‎src/main/java/com/adobe/epubcheck/ops/OPSHandler.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,6 @@ else if (name.equals("script"))
290290
checkScript();
291291
}
292292

293-
resourceType = XRefChecker.Type.HYPERLINK;
294-
295293
String style = e.getAttribute("style");
296294
if (style != null && style.length() > 0)
297295
{

‎src/main/java/com/adobe/epubcheck/overlay/OverlayHandler.java‎

Lines changed: 39 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.adobe.epubcheck.opf.OPFChecker30;
1313
import com.adobe.epubcheck.opf.ValidationContext;
1414
import com.adobe.epubcheck.opf.XRefChecker;
15+
import com.adobe.epubcheck.opf.XRefChecker.Type;
1516
import com.adobe.epubcheck.util.EpubConstants;
1617
import com.adobe.epubcheck.vocab.AggregateVocab;
1718
import com.adobe.epubcheck.vocab.PackageVocabs;
@@ -22,7 +23,6 @@
2223
import com.adobe.epubcheck.vocab.VocabUtil;
2324
import com.adobe.epubcheck.xml.handlers.XMLHandler;
2425
import com.adobe.epubcheck.xml.model.XMLElement;
25-
import com.google.common.base.Strings;
2626
import com.google.common.collect.ImmutableMap;
2727
import com.google.common.collect.ImmutableSet;
2828
import com.google.common.collect.Sets;
@@ -54,6 +54,8 @@ public void startElement()
5454
XMLElement e = currentElement();
5555
String name = e.getName();
5656

57+
processGlobalAttrs();
58+
5759
switch (name)
5860
{
5961
case "smil":
@@ -64,8 +66,7 @@ public void startElement()
6466

6567
case "body":
6668
case "seq":
67-
case "par":
68-
processGlobalAttrs();
69+
processTextRef();
6970
break;
7071

7172
case "text":
@@ -140,79 +141,65 @@ private void checkType(String type)
140141

141142
private void processTextSrc()
142143
{
143-
URL srcURL = checkURL(currentElement().getAttribute("src"));
144-
if (srcURL != null)
144+
URL url = checkURL(currentElement().getAttribute("src"));
145+
if (url != null && context.xrefChecker.isPresent())
145146
{
146-
processRef(srcURL, XRefChecker.Type.HYPERLINK);
147-
148-
if (context.xrefChecker.isPresent())
149-
{
150-
context.xrefChecker.get().registerReference(srcURL, XRefChecker.Type.OVERLAY_TEXT_LINK,
151-
location());
152-
}
147+
processContentDocumentLink(url);
153148
}
154149

155150
}
156151

152+
private void processTextRef()
153+
{
154+
URL url = checkURL(
155+
currentElement().getAttributeNS(EpubConstants.EpubTypeNamespaceUri, "textref"));
156+
if (url != null && context.xrefChecker.isPresent())
157+
{
158+
processContentDocumentLink(url);
159+
}
160+
}
161+
157162
private void processAudioSrc()
158163
{
159164

160-
URL srcURL = checkURL(currentElement().getAttribute("src"));
161-
if (srcURL != null)
165+
URL url = checkURL(currentElement().getAttribute("src"));
166+
if (url != null && context.xrefChecker.isPresent())
162167
{
163-
processRef(srcURL, XRefChecker.Type.AUDIO);
168+
// check that the audio type is a core media type resource
169+
String mimeType = context.xrefChecker.get().getMimeType(url);
170+
if (mimeType != null && !OPFChecker30.isBlessedAudioType(mimeType))
171+
{
172+
report.message(MessageId.MED_005, location(), context.relativize(url), mimeType);
173+
}
174+
175+
// register the URL for cross-reference checking
176+
context.xrefChecker.get().registerReference(url, Type.AUDIO, location());
164177

165-
if (context.isRemote(srcURL))
178+
// if needed, register we found a remote resource
179+
if (context.isRemote(url))
166180
{
167181
requiredProperties.add(ITEM_PROPERTIES.REMOTE_RESOURCES);
168182
}
169183
}
170184
}
171185

172-
private void processRef(URL ref, XRefChecker.Type type)
186+
private void processContentDocumentLink(URL url)
173187
{
174-
assert ref != null;
175-
if (context.xrefChecker.isPresent())
188+
assert url != null;
189+
assert context.xrefChecker.isPresent();
190+
assert context.overlayTextChecker.isPresent();
191+
URL documentURL = URLUtils.docURL(url);
192+
if (!context.overlayTextChecker.get().registerOverlay(documentURL,
193+
context.opfItem.get().getId()))
176194
{
177-
if (type == XRefChecker.Type.AUDIO)
178-
{
179-
String mimeType = context.xrefChecker.get().getMimeType(ref);
180-
if (mimeType != null && !OPFChecker30.isBlessedAudioType(mimeType))
181-
{
182-
report.message(MessageId.MED_005, location(), context.relativize(ref), mimeType);
183-
}
184-
}
185-
else
186-
{
187-
checkFragment(ref);
188-
URL resourceURL = URLUtils.docURL(ref);
189-
// FIXME 2022 see if test case is needed
190-
// if (!Strings.isNullOrEmpty(uniqueResource))
191-
// {
192-
// (uniqueResource was ref-minus-fragment string)
193-
// OverlayTextChecker must be present if XRefChecker is also present
194-
assert context.overlayTextChecker.isPresent();
195-
if (!context.overlayTextChecker.get().registerOverlay(resourceURL, context.opfItem.get().getId()))
196-
{
197-
report.message(MessageId.MED_011, location(), context.relativize(ref));
198-
}
199-
// }
200-
}
201-
context.xrefChecker.get().registerReference(ref, type, location());
195+
report.message(MessageId.MED_011, location(), context.relativize(url));
202196
}
197+
context.xrefChecker.get().registerReference(url, Type.OVERLAY_TEXT_LINK, location());
203198
}
204199

205200
private void processGlobalAttrs()
206201
{
207202
XMLElement e = currentElement();
208-
if (!e.getName().equals("audio"))
209-
{
210-
URL textrefURL = checkURL(e.getAttributeNS(EpubConstants.EpubTypeNamespaceUri, "textref"));
211-
if (textrefURL != null)
212-
{
213-
processRef(textrefURL, XRefChecker.Type.HYPERLINK);
214-
}
215-
}
216203
checkType(e.getAttributeNS(EpubConstants.EpubTypeNamespaceUri, "type"));
217204
}
218205

@@ -238,17 +225,6 @@ private void checkItemReferences()
238225

239226
}
240227

241-
private void checkFragment(URL url)
242-
{
243-
String fragment = url.fragment();
244-
245-
if (Strings.isNullOrEmpty(fragment))
246-
{
247-
// must include a non-empty fragid
248-
report.message(MessageId.MED_014, location());
249-
}
250-
}
251-
252228
protected void checkProperties()
253229
{
254230
if (!context.container.isPresent()) // single file validation

‎src/main/java/com/adobe/epubcheck/overlay/OverlayTextChecker.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ public boolean registerOverlay(URL contentDocURL, String overlayID)
2424
}
2525
else
2626
{
27-
// TODO check if case must really be ignored
28-
return overlayID.equalsIgnoreCase(docToOverlayMap.get(contentDocURL));
27+
return overlayID.equals(docToOverlayMap.get(contentDocURL));
2928
}
3029
}
3130

@@ -40,7 +39,6 @@ public boolean isReferencedByOverlay(URL contentDocURL)
4039

4140
public boolean isCorrectOverlay(URL contentDocURL, String overlayID)
4241
{
43-
// TODO check if case must really be ignored
44-
return overlayID.equalsIgnoreCase(docToOverlayMap.get(contentDocURL));
42+
return overlayID.equals(docToOverlayMap.get(contentDocURL));
4543
}
4644
}

‎src/main/resources/com/adobe/epubcheck/messages/MessageBundle.properties‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,9 @@ MED_012=The "media-overlay" attribute does not match the ID of the Media Overlay
144144
MED_013=Media Overlay Document referenced from the "media-overlay" attribute does not contain a reference to this Content Document.
145145
MED_014=A non-empty fragment identifier is required.
146146
MED_015=Media overlay text references must be in reading order. Text target "%1$s" is before the previous link target in %2$s order.
147-
MED_016=Media Overlays total duration should be the sum of the durations of all Media Overlays documents.
147+
MED_016=Media Overlays total duration should be the sum of the durations of all Media Overlays documents.
148+
MED_017=URL fragment should indicate an element ID, but found '#%1$s'.
149+
MED_018=URL fragment should be an SVG fragment identifier, but found '#%1$s'.
148150

149151
#NAV EPUB v3 Table of contents
150152
NAV_001=The nav file is not supported for EPUB v2.

src/test/resources/epub3/09-media-overlays/files/mediaoverlays-fragid-invalid-error/EPUB/content_001.mp3 renamed to src/test/resources/epub3/09-media-overlays/files/mediaoverlays-textref-fragment-schemebased-warning/EPUB/content_001.mp3

File renamed without changes.

‎src/test/resources/epub3/09-media-overlays/files/mediaoverlays-textref-fragment-schemebased-warning/EPUB/content_001.smil‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<smil xmlns="http://www.w3.org/ns/SMIL" xmlns:epub="http://www.idpf.org/2007/ops" version="3.0">
3+
<body>
4+
<par id="par1">
5+
<text src="content_001.xhtml#xpointer(id('c01'))"/>
6+
<audio src="content_001.mp3"/>
7+
</par>
8+
</body>
9+
</smil>

0 commit comments

Comments
 (0)