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
| Comment | File | Size | Author |
|---|---|---|---|
| #39 | book_export-1189834-d7-39.patch | 1.18 KB | albert volkman |
| #37 | book_export-1189834-d7-37.patch | 746 bytes | albert volkman |
| #31 | 1189822-book-export-31.patch | 1.89 KB | aspilicious |
| #29 | 1189822-book-export-29.patch | 2.05 KB | aspilicious |
| #27 | 1189822-book-export-27.patch | 1.92 KB | aspilicious |
Comments
Comment #1
cosmicdreams commentedshould the effort to assign the book entities microdata be apart of this effort?
http://schema.org/Book
Comment #2
scor commentedI 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.
Comment #3
chrispomeroy commentedchanges 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; ?>">Comment #4
chrispomeroy commentedComment #5
chrispomeroy commentedComment #6
chrispomeroy commentedfixed the patch.
see also #1221724: Convert book-node-export-html.tpl.php to HTML5
Comment #8
Jeff Burnz commentedI 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
Comment #9
rasskull commentedPer Jacine's request, I'm marking that I'd like to help review this patch
Comment #10
jacineTagging this for the next sprint.
Comment #11
chrispomeroy commentedadded /core path
Comment #13
aspilicious commentedtests are failing because of $html_attributes...
Where did this come from and do we need it?
27 days to next Drupal core point release.
Comment #14
chrispomeroy commentedI think it outputs the RDF stuff - I copied it from html.tpl.php
Should I take it out or should we fix the test?
Comment #15
aspilicious commentedThis 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
Comment #17
gábor hojtsyThis 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.
Comment #18
gábor hojtsyComment #19
cosmicdreams commentedComment #20
cosmicdreams commentedSo 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).
Comment #21
gábor hojtsy@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.
Comment #22
cosmicdreams commented@Gabor ah thank you I didn't notice that. I'll test again tonight.
Comment #23
aspilicious commentedLet's try this testbot
Comment #24
aspilicious commented#23: 1189822-book-export.patch queued for re-testing.
Comment #25
aspilicious commentedI think this will fail. ->language is ->langcode now
Comment #27
aspilicious commentedComment #29
aspilicious commentedDAMNIT! I hope this one works :)
Comment #31
aspilicious commentedStupid me. Let's try this one.
Comment #32
aspilicious commentedComment #33
gábor hojtsyPatch achieves the base goal outlined, further cleanups can happen in followups AFAIS.
Comment #34
dries commentedCommitted to 8.x. This language part (not the HML5 part) probably needs a backport to 7.x.
Comment #35
gábor hojtsyI think the extent of the backportability of this patch is something like this (hand edited diff):
(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?
Comment #36
gábor hojtsy(Also, D7 uses $language->language, not $language->langcode notably).
Comment #37
albert volkman commentedLast (D7) patch of the day!
Comment #39
albert volkman commentedGot lazy on friday evening and forgot to run my tests locally. Give this a try.
Comment #40
albert volkman commentedComment #41
aspilicious commentedGood to go
Comment #42
webchickCommitted 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.
Comment #43
gábor hojtsyShould be off-sprint then.