This one is interesting because its for the printed version of book pages, so needs some thought and research about html5 is supported by browsers for print (especially legacy browsers).

http://api.drupal.org/api/drupal/modules--book--book-export-html.tpl.php/7

CommentFileSizeAuthor
#39 book_export-1189834-d7-39.patch1.18 KBAlbert Volkman
#37 book_export-1189834-d7-37.patch746 bytesAlbert Volkman
#31 1189822-book-export-31.patch1.89 KBaspilicious
#29 1189822-book-export-29.patch2.05 KBaspilicious
#27 1189822-book-export-27.patch1.92 KBaspilicious
#23 1189822-book-export.patch1.92 KBaspilicious
#15 book-template.patch4.62 KBaspilicious
#11 book-export-html.tpl_.php-html5-1189834-11.patch691 byteschrispomeroy
#4 book-export-html.tpl_.php-html5-1189834-4.patch4.26 KBchrispomeroy
#6 book-export-html.tpl_.php-html5-1189834-4.patch681 byteschrispomeroy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams commented

should the effort to assign the book entities microdata be apart of this effort?

http://schema.org/Book

scor’s picture

scor CreditAttribution: scor commented

I would keep the structured data (microdata and mappings to schema.org) separate from the purely HTML5 markup. Also, bear in mind that like in Drupal 7, the syntax (RDFa/microdata) should remain distinct from the schema mappings (schema.org or FOAF, or OGP). That's why in D7, the schema is stored in the rdf_mapping table, and the syntax is dealt with in the theme layer with preprocess functions. In other words, you don't want to hard code schema.org references into the theme layer, that's something themers should not have to see or worry about.

chrispomeroy’s picture

chrispomeroy CreditAttribution: chrispomeroy commented
Status: Needs review » Active

changes to doctype and html tag needed. Anything else?

I don't think this DIV below needs to change, as it appears it's only for styling and has no semantic meaning.

<div class="section-<?php print $i; ?>">

chrispomeroy’s picture

chrispomeroy CreditAttribution: chrispomeroy commented
FileSize
book-export-html.tpl_.php-html5-1189834-4.patch4.26 KB
chrispomeroy’s picture

chrispomeroy CreditAttribution: chrispomeroy commented
Status: Active » Needs review
chrispomeroy’s picture

chrispomeroy CreditAttribution: chrispomeroy commented
FileSize
book-export-html.tpl_.php-html5-1189834-4.patch681 bytes

fixed the patch.

see also #1221724: Convert book-node-export-html.tpl.php to HTML5

Status: Active » Needs work

The last submitted patch, book-export-html.tpl_.php-html5-1189834-4.patch, failed testing.

Jeff Burnz’s picture

Jeff Burnz CreditAttribution: Jeff Burnz commented
Status: Needs work » Postponed

I really think we need to wait for html and page templates to finalize before moving on this, or even if we actually even need this template.

So waiting on:
#1077566: Convert html.tpl.php to HTML5
#1077578: [Followup] Convert bartiks page.tpl.php to HTML5

rasskull’s picture

rasskull CreditAttribution: rasskull commented

Per Jacine's request, I'm marking that I'd like to help review this patch

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented
Status: Postponed » Needs work
Issue tags: +sprint

Tagging this for the next sprint.

chrispomeroy’s picture

chrispomeroy CreditAttribution: chrispomeroy commented
Status: Needs work » Needs review
FileSize
book-export-html.tpl_.php-html5-1189834-11.patch691 bytes

added /core path

Status: Needs review » Needs work

The last submitted patch, book-export-html.tpl_.php-html5-1189834-11.patch, failed testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented 
+++ b/core/modules/book/book-export-html.tpl.phpundefined
@@ -16,8 +16,8 @@
+<html<?php print $html_attributes; ?>>

tests are failing because of $html_attributes...

Where did this come from and do we need it?

27 days to next Drupal core point release.

chrispomeroy’s picture

chrispomeroy CreditAttribution: chrispomeroy commented

I think it outputs the RDF stuff - I copied it from html.tpl.php

Should I take it out or should we fix the test?

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs work » Needs review
FileSize
book-template.patch4.62 KB

This should pass (if the suggestion thingie works and I implemented it correct).
But I'm not sure this is all we need for book module...

Needs review to test

Status: Needs review » Needs work

The last submitted patch, book-template.patch, failed testing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

This looks like a great improvement, and would fix xml:lang to be lang via $html_attributes. Yes, we need it. Just like #1330922: Drupal 8 HTML 5 page template should use lang not xml:lang.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: +D8MI
cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams commented
Assigned: Unassigned » cosmicdreams
cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams commented

So I started working on this tonight but wanted to see where the patch was at through manually testing it. I was unable to get anything to come up by looking at the source of what this template is supposed to generate.

I expect to see something, anything. Am I doing it wrong?

EDIT:

In checking my error logs I found this:

Warning: include(/home/blah/drupal/core/modules/book/book-export.tpl.php): failed to open stream: No such file or directory in theme_render_template() (line 1395 of /home/blah/drupal/core/includes/theme.inc).
Warning: include(): Failed opening '/home/blah/drupal/core/modules/book/book-export.tpl.php' for inclusion (include_path='.:/usr/share/php:/usr/share/pear') in theme_render_template() (line 1395 of /home/blah/core/includes/theme.inc).

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

@cosmicdreams: well, the patch from @aspilicious renames the file, so you should rebuild your theme cache after applying the patch. The error messages posted indicate your system is still looking for the old file.

cosmicdreams’s picture

cosmicdreams CreditAttribution: cosmicdreams commented

@Gabor ah thank you I didn't notice that. I'll test again tonight.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs work » Needs review
FileSize
1189822-book-export.patch1.92 KB

Let's try this testbot

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

#23: 1189822-book-export.patch queued for re-testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

I think this will fail. ->language is ->langcode now

Status: Needs review » Needs work

The last submitted patch, 1189822-book-export.patch, failed testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs work » Needs review
FileSize
1189822-book-export-27.patch1.92 KB

Status: Needs review » Needs work

The last submitted patch, 1189822-book-export-27.patch, failed testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs work » Needs review
FileSize
1189822-book-export-29.patch2.05 KB

DAMNIT! I hope this one works :)

Status: Needs review » Needs work

The last submitted patch, 1189822-book-export-29.patch, failed testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
FileSize
1189822-book-export-31.patch1.89 KB

Stupid me. Let's try this one.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs work » Needs review
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Patch achieves the base goal outlined, further cleanups can happen in followups AFAIS.

Dries’s picture

Dries CreditAttribution: Dries commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed to 8.x. This language part (not the HML5 part) probably needs a backport to 7.x.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

I think the extent of the backportability of this patch is something like this (hand edited diff):

-<html xmlns="http://www.w3.org/1999/xhtml" lang="<?php print $language->langcode; ?>" xml:lang="<?php print $language->langcode; ?>">
+<html xmlns="http://www.w3.org/1999/xhtml" lang="<?php print $language->langcode; ?>" xml:lang="<?php print $language->langcode; ?>" dir="<?php print $language->dir; ?>">

(Also $language->dir is prefilled in the page template code to the right ltr/rtl designation, not sure that it actually happens in a preprocess here ATM). In effect, the missing dir value should be added. Otherwise changing the book export template to use $html_attributes would be a feature change for theme writers, no?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

(Also, D7 uses $language->language, not $language->langcode notably).

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
Status: Patch (to be ported) » Needs review
FileSize
book_export-1189834-d7-37.patch746 bytes

Last (D7) patch of the day!

Status: Needs review » Needs work

The last submitted patch, book_export-1189834-d7-37.patch, failed testing.

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
FileSize
book_export-1189834-d7-39.patch1.18 KB

Got lazy on friday evening and forgot to run my tests locally. Give this a try.

Albert Volkman’s picture

Albert Volkman CreditAttribution: Albert Volkman commented
Status: Needs work » Needs review
aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs review » Reviewed & tested by the community

Good to go

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. The commit message says "Issue #1189834 by aspilicious, chrispomeroy, Albert Volkman: Add language direction to book-export-html.tpl.php" since that seemed to be the only thing this was about.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: -sprint

Should be off-sprint then.

Automatically closed -- issue fixed for 2 weeks with no activity.