Problem/Motivation

Book module renders export pages with custom page template that should contain "print.css" but this broken because asset hardcoded relatively to current page.
Also there's no way to alter assets attached to the page because response is returned from controller without processing assets (to prevent unneeded styling)

Steps to reproduce:
1) enable book module and create new book node
2) visit "Printer-friendly version" like /book/export/html/1
3) make sure print.css loaded

currently it's broken because path should be core/misc/print.css
print-css

Proposed resolution

Make new library core/drupal.print with print.css
Allow theme libraries to override/extend drupal.print library
Add tests and commit

Remaining tasks

find a way to render attached libraries in custom responses that should skip page template from active theme

User interface changes

libraries attached in book export pages should be rendered

API changes

no

Original report by [@pianomansam]

The book module should really have the capability to pull in the print stylesheets from the active theme. Right now it is hardcoded in book-export-html.tpl.php to only pull in misc/print.css and getting it to do more requires copying the template file into the theme directory and manually pulling in a print stylesheet. This is very hackish and uncharacteristic of the Drupal Way.

Files: 
CommentFileSizeAuthor
#23 1338858_23.patch3.97 KBMile23
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 103,197 pass(es), 33 fail(s), and 0 exception(s). View
#9 1338858-print-css-9.patch2.73 KBandypost
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,767 pass(es), 0 fail(s), and 24 exception(s). View

Comments

watbe’s picture

Version: 7.9 » 7.19
Assigned: Unassigned » watbe

Just encountered this issue. Should be a simple fix.

watbe’s picture

Version: 7.19 » 8.x-dev
Assigned: watbe » Unassigned
Issue tags: +Newbie, +needs backport to D7

I can't find the function that would load a theme's defined stylesheet. Is that even possible? Need to load the stylesheets defined in a theme's .info as [print].

Updating tags.

watbe’s picture

Issue summary: View changes

correcting typo

clemens.tolboom’s picture

Current implementation is still the same but now as a twig template.

core/modules/book/templates/book-export-html.html.twig:    <link type="text/css" rel="stylesheet" href="misc/print.css" />
andypost’s picture

Category: Feature request » Bug report
Issue summary: View changes

This is a bug, discovered working on #2477223: Refactor _drupal_add_html_head, drupal_get_html_head, _drupal_add_html_head_link into the attachments processor, remove from common.inc.

git grep print.css

core/modules/book/templates/book-export-html.html.twig:27:    <link type="text/css" rel="stylesheet" href="misc/print.css" />
core/themes/classy/templates/layout/book-export-html.html.twig:25:    <link type="text/css" rel="stylesheet" href="misc/print.css" />

So templates hardcodes wrong path

jaimeguzman’s picture

Status: Active » Needs review

We found this bug with andypost try it to fix this: https://www.drupal.org/node/2477223#comment-9922524

The book export show a missed path for css, also I think that not have to showed.

diff --git a/core/modules/book/templates/book-export-html.html.twig b/core/modules/book/templates/book-export-html.html.twig
index 5fb2e97..5813af8 100644
--- a/core/modules/book/templates/book-export-html.html.twig
+++ b/core/modules/book/templates/book-export-html.html.twig
@@ -24,7 +24,7 @@
     <title>{{ title }}</title>
     {{ page.head }}
     <base href="{{ base_url }}" />
-    <link type="text/css" rel="stylesheet" href="misc/print.css" />
+    <link type="text/css" rel="stylesheet" href="core/misc/print.css" />
   </head>
   <body>
     {#
diff --git a/core/themes/classy/templates/layout/book-export-html.html.twig b/core/themes/classy/templates/layout/book-export-html.html.twig
index ea33648..8d3c6ac 100644
--- a/core/themes/classy/templates/layout/book-export-html.html.twig
+++ b/core/themes/classy/templates/layout/book-export-html.html.twig
@@ -22,7 +22,7 @@
     <title>{{ title }}</title>
     {{ page.head }}
     <base href="{{ base_url }}" />
-    <link type="text/css" rel="stylesheet" href="misc/print.css" />
+    <link type="text/css" rel="stylesheet" href="core/misc/print.css" />
   </head>
   <body>
     {#
clemens.tolboom’s picture

Status: Needs review » Needs work

@jaimeguzman the original poster want drupal to include a print.css from the current theme.

Please create a https://www.drupal.org/patch file :)

andypost’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Newbie +Needs tests
FileSize
32.15 KB
2.71 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,774 pass(es), 0 fail(s), and 24 exception(s). View

Updated IS with current state.
The actual problem that there's no library that themes can override/extend with custom css also because assets are not processed.

Book module can't execute template_preprocess_page() to prevent other assets attached.

WIP patch, still broken.

andypost’s picture

FileSize
333 bytes
2.73 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,767 pass(es), 0 fail(s), and 24 exception(s). View

So only media: print assets should be rendered

andypost’s picture

Issue tags: +Twig, +WSCCI

The last submitted patch, 8: 1338858-print-css-8.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 9: 1338858-print-css-9.patch, failed testing.

Wim Leers’s picture

+++ b/core/modules/book/src/Controller/BookController.php
@@ -154,7 +154,12 @@ public function bookExport($type, NodeInterface $node) {
-    return new Response(drupal_render($exported_book));
+    // @todo Does not work!
+    drupal_process_attached($exported_book);
+    // @todo Probably renderRoot() is not needed here.
+    $content = \Drupal::service('renderer')->renderRoot($exported_book);
+    $responce = new Response($content);
+    return $responce;

This cannot possibly work because the HTML main content renderer is not involved, which means a lot is bypassed.

I'll need to debug this to figure out how this actually should be implemented.

dawehner’s picture

+++ b/core/modules/book/book.module
@@ -478,6 +478,7 @@ function template_preprocess_book_export_html(&$variables) {
+  // @todo Populate HEAD somehow.
   $variables['head'] = drupal_get_html_head();

So what are we trying to achieve here?

andypost’s picture

As I get current render I need to run a main content rendered on that custom template.
Otoh there some SimplePageVariant to exclude blocks.

Currently book export render mostly broken because it was forgotten to to update with current render/cache changes, so maybe better to #1843718: Remove "export" functionality from Book module, apply to all node types at the current stage

Crell’s picture

This sounds like another use case for a custom rendering context. Have a look at Aggregator module, which I think is the only other example in core. (Rendering context is not currently an explicit thing, although it probably should be.) You have to build a render array and call the renderer yourself. Do NOT call any render/markup functions directly, as those are all vestigial and will hopefully die soon.

andypost’s picture

The only example I found in core is \Drupal\views\Plugin\views\display\Feed::buildResponse and template_preprocess_views_view_rss

So looks here we need to do the same CacheableResponse that uses custom render and custom asset attach...

@Crell there's no "rendering context" in core yet

andypost’s picture

Another way is to use bare_html_page_renderer service!
\Drupal\system\Controller\DbUpdateController::handle() uses that one

Crell’s picture

Crell’s picture

Issue tags: -WSCCI

The API rendering context issue is in, so this issue should be doable now.

joelpittet’s picture

Issue tags: +CSS
Mile23’s picture

Another way is to use bare_html_page_renderer service!

If by 'another' you mean "...in addition to the renderer service..." then yes. These should be handled through the available services, and no other way. If there's no way to do that, then the solution is to fix the services so they can provide the use-case.

Also, related: #2477223: Refactor _drupal_add_html_head, drupal_get_html_head, _drupal_add_html_head_link into the attachments processor, remove from common.inc. has to comment out a line from BookTest to pass the testbot, so we're in a bit of a holding pattern on this one.

Mile23’s picture

Status: Needs work » Needs review
FileSize
3.97 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 103,197 pass(es), 33 fail(s), and 0 exception(s). View

This patch sort of halfway works. :-)

We have BookController use the bare_html_page_renderer service to generate a Response object and return that. I'm under the impression that a Response object should bypass normal theming when returned from a controller.

However this led to the content being themed twice, so I deleted the twig file from bartik. I figure this is OK since the premise here is that the book export should have its own theme anyway.

The big problem, however, is that something is outputting the HTTP headers as part of the content. I assume that's the kernel getting mixed up in some way, and not a problem with book.

And of course the css itself could probably use some love, but I can't get far enough to address that.

Also I just started working without all the proper rigamarole so I don't have an interdiff. Sorry.

Status: Needs review » Needs work

The last submitted patch, 23: 1338858_23.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mgifford’s picture

Issue tags: +print.css

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.