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

Add note about the problem accessing resources #1724

Merged
merged 4 commits into from
Jul 8, 2021
Merged

Add note about the problem accessing resources #1724

merged 4 commits into from
Jul 8, 2021

Conversation

mattgarrish
Copy link
Member

@mattgarrish mattgarrish commented Jun 28, 2021

I've added a short note about the problem to the OCF section per the resolution in #1687 and also expanded on the note in the multiple renditions spec since that's really where this issue would come into play.

Fixes #1687


Preview | Diff

…rectory containing the package document;

expand on the multiple renditions note to explain the issue in more detail
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 proposed note looks OK to me.

But I'm also wondering why we do not put this as a SHOULD?
After all, the core spec is about defining authoring recommendations. Here we have a know source of interoperability issues, so a normative SHOULD seems appropriate. To adapt RFC2119 language: there may exist valid reasons to put resources in a sibling directory, but the full implications for interoperability must be understood and carefully weighed before doing so.

Unless we're afraid that EPUBCheck-warning-means-rejection will cause havoc, of course.

@iherman
Copy link
Member

iherman commented Jun 29, 2021

In my view, we are documenting here a misbehavior of some Reading Systems. It is still pending whether we would normatively restrict (even with a SHOULD) the folder structure in the core document. Until that does happen, I would feel uncomfortable putting this as a SHOULD in this spec.

(We can always update that later)

@mattgarrish
Copy link
Member Author

mattgarrish commented Jun 29, 2021

Unless we're afraid that EPUBCheck-warning-means-rejection will cause havoc, of course.

It probably won't cause any actual havoc, but there is concern about making this a warning if we don't need to. We're not sure how far we want to get into normatively hammering authors or developers with this problem when, outside of developing multiple renditions, no one has reported it as a real-world issue they've encountered so far.

There's also that the fix, as @iherman notes, should really be on the reading system side, but there's concern there, too, that even if we make a change now it won't lead to reading systems being changed. It looks like an optimization based on the assumption that the vast majority of content is already self-contained in an /OEBPS or /OPS or /EPUB directory.

So, in effect, we're just documenting what we know about the problem for now (with the option to warn authors as an info message in epubcheck, I suppose). The idea was that we could return to this in the future if it becomes more of an issue.

@rdeltour
Copy link
Member

OK fine by me. I was thinking we could pave that cowpath in the spec, but if we think it's not ready to be paved or that there's hope that the RS situation will make that useless in the future, then a note is certainly enough 😊.

@iherman
Copy link
Member

iherman commented Jun 29, 2021

OK fine by me. I was thinking we could pave that cowpath in the spec, but if we think it's not ready to be paved or that there's hope that the RS situation will make that useless in the future, then a note is certainly enough 😊.

I would hope for the latter...

@mattgarrish
Copy link
Member Author

Are we waiting on anything specific before merging this? (i.e., nothing in last week's call negated this PR, did it?)

@dauwhe
Copy link
Contributor

dauwhe commented Jul 8, 2021

Are we waiting on anything specific before merging this? (i.e., nothing in last week's call negated this PR, did it?)

I think we should merge. It's a step in the right direction; we may take more steps.

@iherman
Copy link
Member

iherman commented Jul 8, 2021

My apologies, I forgot to add my 👍 in the approvals. Done now, I agree to merge.

@dauwhe dauwhe merged commit 0e64b4e into main Jul 8, 2021
@dauwhe dauwhe deleted the fix/issue-1687 branch July 8, 2021 14:26
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.

Refine the requirements on how RS must process the container directory structure
4 participants