More fixes from examples.

nlbdev / nordic-epub3-dtbook-migrator

Tools for converting between a strict subset of DTBook and EPUB3.

http://nlbdev.github.io/nordic-epub3-dtbook-migrator/

GNU Lesser General Public License v2.1

8 stars 7 forks source link

More fixes from examples. #435

Closed kalaspuffar closed 3 years ago

kalaspuffar commented 3 years ago

Hi @josteinaj

This package of fixes contains:

Allow role, aria-label and aria-labelledby
remove DTBook legacy docauthor and doctitle "The validator still seems to expect a single-page structure. I think this is in the RelaxNG file." - @martinpub
revert version to 3.0. "This is incorrect, it should be 3.0 also for EPUB 3.2, see https://www.w3.org/publishing/epub3/epub-packages.html#sec-package-elem." - @martinpub
Allow text in page breaks

The above change where we remove the header block is what I interpreted @martinpub's comment above. I understand that we don't want this legacy in the code when it's not specified in the documentation but I'm not familiar with the "single-page structure" so I did not know exactly what was required by this comment.

Best regards Daniel

josteinaj commented 3 years ago

The legacy single-page structure was:

<html>
  <head>…</head>
  <body>
    <header><!-- doctitle / docauthor --></header>
    <section><!-- chapter 1 --></section>
    <section><!-- chapter 2 --></section>
  </body>
</html>

And the equivalent multipage structure was:

<html>
  <head>…</head>
  <body><!-- chapter 1 --></body>
</html>

<html>
  <head>…</head>
  <body><!-- chapter 2 --></body>
</html>

The <header> element mapped the <doctitle>, <docauthor> and <covertitle> elements from DTBook to HTML, but also functioned as a sort of "flag" for validation rules to see if it was a single-page HTML file or a multi-page HTML file. No <header> means multi-page. In the schematrons you probably noticed several matches for body[header] which is a way of matching only body elements in single-document versions.

kalaspuffar commented 3 years ago

Hi @josteinaj

I've implemented your suggestions. Please review again.

Best regards Daniel

kalaspuffar commented 3 years ago

Hi @josteinaj

Great suggestion, I've changed describedby to be a more general implementation. Please review again.

Best regards Daniel

josteinaj commented 3 years ago

The only thing remaining is https://github.com/nlbdev/nordic-epub3-dtbook-migrator/pull/435#discussion_r596825648

@martinpub @AndersEkl This rule was too strict:

[nordic105]: Pagebreaks must not contain anything

So we could remove it completely, but I suggest that we instead replace it with:

[nordic105]: Pagebreaks must not contain elements or comments. Text content, if present, must not contain extra whitespace.

Would you be fine with that?

josteinaj commented 3 years ago

@kalaspuffar I approved this for merging a bit prematurely, sorry. I thought I only approved some of the changes and not the whole PR, but maybe that's not how reviews work on Github. I should have mentioned the question about nordic105 before approving.

But it's not a big deal. I see you included the change, which I think is good. If @martinpub or @AndersEkl wants to remove it completely it could be removed in a later PR.

kalaspuffar commented 3 years ago

Hi @josteinaj

I've added that rule to the repository and I think it seems reasonable to not allow empty tags or just spaces.

Best regards Daniel

AndersEkl commented 3 years ago

The only thing remaining is #435 (comment)

@martinpub @AndersEkl This rule was too strict:
* `[nordic105]`: _Pagebreaks must not contain anything_
So we could remove it completely, but I suggest that we instead replace it with:
* `[nordic105]`: _Pagebreaks must not contain elements or comments. Text content, if present, must not contain extra whitespace._
Would you be fine with that?

Seems reasonable to me, yes.

martinpub commented 3 years ago

Seems reasonable to me, yes.

Agreed!