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

Remove element restrictions from remote resource locations #1943

Merged
merged 3 commits into from
Dec 3, 2021

Conversation

mattgarrish
Copy link
Member

@mattgarrish mattgarrish commented Nov 30, 2021

Although there's a security advantage in saying where remote resources can be used, rather than relying solely on media type to allow them (which people periodically try to fake to get through epubcheck), this PR undoes our earlier change because I don't see a simple way to account for custom elements or dynamic writing of content generally.

Trying to be overly-restrictive always has a way of coming back and biting us, too, when some new technique comes along that we haven't accounted for, so this is probably the better long-term option.

I also moved up the notes that were after the examples, as it was a weird place for them (usually we put examples last). I also removed the note formatting from the one that says the rules apply to both CMT and foreign resource types as it is more of a normative statement than an informative one.

Fixes #1913


Preview | Diff

@iherman
Copy link
Member

iherman commented Dec 3, 2021

The issue was discussed in a meeting on 2021-12-03

List of resolutions:

View the transcript

1. Custom elements and remote resources (issue epub-specs#1913)

See github issue epub-specs#1913.

Dave Cramer: first few issues are about remote resources.

See github pull request epub-specs#1949.

See github pull request epub-specs#1943.

Dave Cramer: all of our previous conclusions are complicated by custom elements in HTML.
… mgarrish tries to phrase our existing requirements in a way that fits with custom elements.
… but this makes things impossible for epubcheck, which won't know what is actually there until scripts have run.

Matt Garrish: right, there are 2 PRs, this one tries to find language that still works with our past intent, the other PR more specifically address the existence of custom elements.
… there is really little we can do to check for this, people will try to work around it. It will ultimately be down to RS to decide whether to even support custom elements.

Dave Cramer: i made a test book for this, it worked in Apple and ADE, Kindle previewer failed because it doesn't support js, didn't work in Thorium.

Ivan Herman: what you tested tells me that custom elements are here, it would be foolish of us to try to restrict them. They are part of the element that we have to deal with.
… I'm in favor of the first PR, which removes restrictions. The second PR which tries to keep restrictions is harder to check anyway.

Ivan Herman: +1 to matt.

Matt Garrish: the simpler solution might be just to say that RS should not render remote resources in iframes.

Dave Cramer: worried that people would then work around that by using a custom element to effectively make an iframe.

Matt Garrish: there's more awareness of how to handle that at the RS level than at checking, because you have the DOM.

Brady Duga: when it comes to scripting, it kind of the wild west. It'll be impossible to test whether scripts are being used to do something malicious.
… there's utility to having everything be in the package (interop, archival), and if people don't care about that then there's little we can do.
… our main concern in making these exceptions for resources are for size.
… should the restriction be based on size then.

Ivan Herman: to be clear, custom elements mostly means scripting. As soon as RS allows scripting, any restriction on custom elements is meaningless. The real restriction is on scripting.
… the first PR simplifies that.

Dave Cramer: there are 2 types of custom elements. First inherents from existing HTML element (i.e. "is" attribute). This type doesn't work in webkit, and will probably be limited in epub world..
… this is a can of worms that we can't really do much about.

Proposed resolution: Merge PR 1943 and not 1949. (Wendy Reid)

Dave Cramer: +1.

Ivan Herman: +1.

Ben Schroeter: +1.

Brady Duga: +1.

Wendy Reid: +1.

Matthew Chan: +1.

Toshiaki Koike: +1.

Matt Garrish: +1.

Masakazu Kitahara: +1.

Charles LaPierre: .

Charles LaPierre: ].

George Kerscher: +1.

Dan Lazin: +1.

Charles LaPierre: +1.

Resolution #1: Merge PR 1943 and not 1949.

Bill Kasdorf: +1.

Proposed resolution: Close issue 1913 once PR 1943 is merged.. (Wendy Reid)

Ben Schroeter: +1.

Brady Duga: +1.

Wendy Reid: +1.

Matthew Chan: +1.

Masakazu Kitahara: +1.

Ivan Herman: +1.

Bill Kasdorf: +1.

Dave Cramer: +1.

Dan Lazin: +1.

Matt Garrish: +1.

Charles LaPierre: +1.

Resolution #2: Close issue 1913 once PR 1943 is merged..

Dave Cramer: I would warn everyone to be wary of custom elements, because it's easy to miss things related to a11y. HTML elements take care of a lot of that thinking for you.

Ivan Herman: that's not really our fight, leave for WHAT WG.

Dan Lazin: we use custom elements quite a bit in my day job, and there are more a11y friendly ways of doing it, but yes, beware.

@mattgarrish mattgarrish merged commit 8b8877c into main Dec 3, 2021
@mattgarrish mattgarrish deleted the fix/issue-1913 branch December 3, 2021 16:20
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.

Custom elements and remote resources
4 participants