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.

CommentFileSizeAuthor
#23 1338858_23.patch3.97 KBMile23
#9 1338858-print-css-9.patch2.73 KBandypost
#9 interdiff.txt333 bytesandypost
#8 1338858-print-css-8.patch2.71 KBandypost
#8 print-css.png32.15 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

watbe’s picture

watbe CreditAttribution: watbe commented
Version: 7.9 » 7.19
Assigned: Unassigned » watbe

Just encountered this issue. Should be a simple fix.

watbe’s picture

watbe CreditAttribution: watbe commented
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

watbe CreditAttribution: watbe commented
Issue summary: View changes

correcting typo

clemens.tolboom’s picture

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
CreditAttribution: clemens.tolboom commented

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

andypost
he/him
Primary language Russian
CreditAttribution: andypost at Skilld commented
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

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost at Skilld commented
Related issues: +#1843718: Remove "export" functionality from Book module, apply to all node types, +#2477223: Refactor _drupal_add_html_head, drupal_get_html_head, _drupal_add_html_head_link into the attachments processor, remove from common.inc.
jaimeguzman’s picture

jaimeguzman CreditAttribution: jaimeguzman at Skilld commented
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

clemens.tolboom
Primary language Dutch
Location Groningen, 🇳🇱/🇪🇺
CreditAttribution: clemens.tolboom as a volunteer commented
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

andypost
he/him
Primary language Russian
CreditAttribution: andypost at Skilld commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Newbie +Needs tests
FileSize
print-css.png32.15 KB
1338858-print-css-8.patch2.71 KB

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

andypost
he/him
Primary language Russian
CreditAttribution: andypost at Skilld commented
FileSize
interdiff.txt333 bytes
1338858-print-css-9.patch2.73 KB

So only media: print assets should be rendered

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost at Skilld commented
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

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented 
+++ 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

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented 
+++ 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

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

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

Crell CreditAttribution: Crell at Palantir.net commented

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

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

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

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

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

Crell’s picture

Crell CreditAttribution: Crell at Palantir.net commented
Related issues: +#2450993: Rendered Cache Metadata created during the main controller request gets lost

Related, which will provide a clearer API for render context: #2450993: Rendered Cache Metadata created during the main controller request gets lost

Crell’s picture

Crell CreditAttribution: Crell at Palantir.net commented
Issue tags: -WSCCI

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

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue tags: +CSS
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

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

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs work » Needs review
FileSize
1338858_23.patch3.97 KB

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

mgifford
he/him
Primary language English
CreditAttribution: mgifford at OpenConcept Consulting Inc. commented
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.

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

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.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.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.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.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.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.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
saphemmy’s picture

saphemmy CreditAttribution: saphemmy at CI&T commented
Assigned: Unassigned » saphemmy

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Status: Needs work » Postponed

This extension is being deprecated, see #3376070: [Meta] Tasks to deprecate Book module. It will be removed from core and moved to a contrib project, #3376101: [11.x] [Meta] Tasks to remove Book.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

This issue may be re-opened if it can be considered critical, If unsure, re-open the issue and ask in a comment.

quietone’s picture

quietone CreditAttribution: quietone at PreviousNext commented
Assigned: saphemmy » Unassigned

Un-assigning per Assigning ownership of a Drupal core issue.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.