Issue status update for http://drupal.org/node/1898 Project: Drupal Version: cvs Component: book.module Category: bug reports -Priority: normal +Priority: critical -Assigned to: al +Assigned to: puregin Reported by: kika Updated by: puregin -Status: active +Status: patch Attachment: http://drupal.org/files/issues/book_printer_friendly_div.patch (1.57 KB) I am attaching a patch which fixes improper printer-friendly output, as discussed. The patch fixes book_print and book_print_recurse() to insert a <div> start tag before emitting the header, and to close this after subsections have been recursively generated. The <div> tag has an id attribute of the form id="sect-123", where the node's nid is 123, and a class attribute of form class="sect/n/" where the depth of the section is /n/. I'm marking this issue as critical, since the present HTML output plays havoc with any attempts at sane PDF generation. I will address the issue of where printer-friendly subsections should be 'rooted' in a follow-up. Djun puregin Previous comments: ------------------------------------------------------------------------ June 9, 2003 - 14:06 : kika Snippet from book_render(): $output .= "<dt>". l($node->title, "node/view/$node->nid") ."</dt><dd>". book_body($node) ."<br /><br /></dd>"; Is using definition list here is really correct? This should be replaced with appropiate div's or even better, funnel through theme() Same goes for the function book_print(): $output .= "$node->title"; if ($node->body) { $output .= "<ul>". book_body($node) ."</ul>"; } Why wrap body inside <ul>? ------------------------------------------------------------------------ June 10, 2003 - 16:31 : al Yeah, it's all nasty. I patched this ages ago, but no one seemed interested. The patch now won't apply due to all the changes since then. I'm on this - expect a new patch to hit my sandbox shortly. ------------------------------------------------------------------------ June 10, 2003 - 22:03 : ax please remove the superfluous <ul>s and <li> [1], too. to solve the problem of invalid nested <hx> tags, maybe we should change <h$depth> to <h1 class="book-h$depth">? this way, we could use <h2> - <h6> in book pages. alternatively, we should only use <h5> and <h6> in book pages - we shouldn't have nestings deeper than 4 in printed books, then. additionally, i think it would be a good idea to add id's to all book headings so they can be jumped to via fragment identifiers (url#fragment-identifier). i often wish this would be possible when referencing the last paragraph of a long page of documentation ... these id's should be of the form <hx id="foo" name="foo"> [2], with foo probably being the node-id of the single book page. adding id's to headings would also be a good idea for headings / sections in single book nodes. for example, it would be nice if i could link to list point 10 in the drupal installation instructions [3] via something like url#10. these id's had to be added manually. the question is: how can these id's be named to be unique within a /printed/ book? probably <node_id>_<section_id> ... [1] [ [drupal-devel] Drupal handbook formatting and heading tags | http://lists.drupal.org/pipermail/drupal-devel/2003-June/025860.html ] [2] [ XHTML Spec, HTML Compatibility Guidelines, Fragment Identifiers | http://www.w3.org/TR/xhtml1/#C_8 ] [3] http://drupal.org/node/view/260 ------------------------------------------------------------------------ June 15, 2003 - 06:23 : al Fixed in latest CVS. Uses all of Ax's suggestions: - <h1 class="depthX"> - Fixed to use CSS rather than tables. - Changes link at top to standard "breadcrumb" style, rather than hierarchy in tree view. - Removes odd and superfluous <ul> tags and the like. - Makes printed docs have proper <html> and <body> wrapping, etc. ------------------------------------------------------------------------ May 4, 2005 - 23:20 : puregin I'm re-opening this, because the problem persists in CVS. 1) The current CVS version (1.288.2.4) of book module still wraps $node->body in an <ul> element in book_print_recurse(). This produces non-conformant HTML. I'm not sure why this was done in the first place - I'm guessing someone wanted to insulate from having bad markup in a book page throw off the enclosing page. Getting rid of the offending UL tags is a one-line solution, but what is the right thing to do here? I'd like to see something very structural - wrapping the /entire/ output (including the generated sectional header) of the book_print_recurse() wrapped in a <div> element. This will properly nest true structural sections, making the structure accessible to applications via DOM. It will also make it easier to use XSLT, for example, to transform into various XML based export formats, for example. Any reason not to do so? Also... when we get the 'printer-friendly' version of a page, the subtree rooted at that page is returned, and the root page is marked up with a class="book-h1". Is there any reason why we don't compute the depth of the root page, and use that to generate the "in-place" header? In other words, if a section is "book-h3" relative to the top-level book, shouldn't it be "book-h3" whenever it is viewed? In this case, we could apply the same principal to generating the <div> tags around each section.