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

Normalize the case of boolean attributes values #941

Closed
francofaa opened this issue Jan 16, 2019 · 5 comments
Closed

Normalize the case of boolean attributes values #941

francofaa opened this issue Jan 16, 2019 · 5 comments
Assignees
Labels
priority: medium To be processed and published in one of the upcoming releases spec: EPUB 3.2 Impacting the support of EPUB 3.2 status: completed Work completed, can be closed type: bug The issue describes a bug
Milestone

Comments

@francofaa
Copy link

epubcheck flags an error for the following attributes on element iframe:

  • allowFullScreen="allowFullScreen"
  • allowfullscreen=""
  • allow="fullscreen"

Error reads as follows:

ERROR(RSC-005): ./Documents/Work/Validator/Testing Blank Files/S_EPUB_AllowFullScreen_ALL.epub/OEBPS/xhtml/jac_9781319048860_back.xhtml(18,106): Error while parsing file: attribute "allowFullScreen" not allowed here; expected attribute...

("..." omits the enumerated allowed attributes on iframe).

Markup from file here for testing:

<iframe src="http://www.youtube.com/embed/xDMP3i36naA" allowFullScreen="allowFullScreen"></iframe>
<iframe src="http://www.youtube.com/embed/xDMP3i36naA" allowfullscreen=""></iframe>
<iframe src="http://www.youtube.com/embed/xDMP3i36naA" allow="fullscreen"></iframe>

HTML validator allows these, so I propose that this may be a bug in epubcheck.

@rdeltour rdeltour self-assigned this Jan 16, 2019
@rdeltour rdeltour added type: bug The issue describes a bug spec: EPUB 3.2 Impacting the support of EPUB 3.2 status: accepted Ready to be further processed priority: medium To be processed and published in one of the upcoming releases labels Jan 16, 2019
@mattgarrish
Copy link
Member

Are you using the alpha 4.2 release or 4.1.0? If 4.1.0, note that the attributes won't be available until EPUB 3.2 support is integrated.

I'm not able to reproduce this in the alpha, though, as both allow and allowfullscreen are defined for iframe in embed.rnc.

@rdeltour
Copy link
Member

I could reproduce on the next/4.2.0 branch. The issue only occurs however with the form allowFullScreen="allowFullScreen"; I suppose it will be the case for all the other boolean attributes. I suppose this is something checked at parsing time by the Nu Html Checker, in order to simplify their schemas.

I'll double-check and add either a parse-time check or fix the schemas where needed. Good catch @francofaa!

@mattgarrish
Copy link
Member

allowFullScreen="allowFullScreen";

Isn't that because it has to be allowfullscreen="allowfullscreen"? That syntax works fine for me.

@rdeltour
Copy link
Member

rdeltour commented Jan 16, 2019

Oh no wait, I replied too fast. Actually the issue is that the attribute name is lower-case, it should be allowfullscreen and not allowFullScreen

@rdeltour
Copy link
Member

In any case, boolean attributes accept values that are case-insensitive matches for their canonical name, so we should perform some case normalization at parsing time.

@rdeltour rdeltour added status: ready for implem The issue is ready to be implemented and removed status: accepted Ready to be further processed labels Jan 16, 2019
@rdeltour rdeltour changed the title full screen attributes not allowed on iframe element Normalize the case of boolean attributes values Jan 16, 2019
rdeltour added a commit that referenced this issue Feb 8, 2019
- add an `HTMUtils` facility to get whether an HTML attribute has a
  case-insensitive value (boolean and enumerated attributes)
- pre-process case-insensitive HTML attributes to lower-case their value
- refactor the pre-processing attribute logic for more clarity
- add a test

Fix #941
@rdeltour rdeltour added this to the 4.2.0-beta milestone Feb 8, 2019
@rdeltour rdeltour added status: has PR The issue is being processed in a pull request and removed status: ready for implem The issue is ready to be implemented labels Feb 8, 2019
rdeltour added a commit that referenced this issue Feb 8, 2019
- add an `HTMUtils` facility to get whether an HTML attribute has a
  case-insensitive value (boolean and enumerated attributes)
- pre-process case-insensitive HTML attributes to lower-case their value
- refactor the pre-processing attribute logic for more clarity
- add a test

Fix #941
@rdeltour rdeltour added status: completed Work completed, can be closed and removed status: has PR The issue is being processed in a pull request labels Feb 8, 2019
@rdeltour rdeltour closed this as completed Feb 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium To be processed and published in one of the upcoming releases spec: EPUB 3.2 Impacting the support of EPUB 3.2 status: completed Work completed, can be closed type: bug The issue describes a bug
Projects
None yet
Development

No branches or pull requests

3 participants