Remove drupal_add_css() from book.module - use #attached instead.

Meta issue: #1839338: [meta] Remove drupal_set_*() drupal_add_*() in favour of #attached/response for out-of-band stuff

Files: 
CommentFileSizeAuthor
#11 2012526-11-remove-drupal_add_css.patch1016 bytesmcjim
PASSED: [[SimpleTest]]: [MySQL] 55,404 pass(es).
[ View ]
#9 2012526-8-remove-drupal_add_css.patch1012 bytesmcjim
PASSED: [[SimpleTest]]: [MySQL] 55,278 pass(es).
[ View ]
#3 2012526-3-remove-drupal_add_css.patch1008 bytesmcjim
PASSED: [[SimpleTest]]: [MySQL] 56,062 pass(es).
[ View ]
#1 2012526-remove-drupal_add_css.patch2.07 KBmcjim
PASSED: [[SimpleTest]]: [MySQL] 55,352 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.07 KB
PASSED: [[SimpleTest]]: [MySQL] 55,352 pass(es).
[ View ]

Removed drupal_add_css from template_preprocess_book_navigation().

CSS is #attached in three different places:

In book_node_view() for the navigation below the node.

In BookNavigationBlock twice, for the block shown on all pages and the block only shown on book pages.

Status:Needs review» Needs work

Actually, scratch that. The CSS is not needed by the blocks at all. Re-rolling…

Status:Needs work» Needs review
StatusFileSize
new1008 bytes
PASSED: [[SimpleTest]]: [MySQL] 56,062 pass(es).
[ View ]

Re-rolled. CSS is #attached only in book_node_view().

Issue tags:+Needs manual testing

Code looks good. This needs manual testing, however, to verify that the CSS is still being applied in all cases.

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs manual testing

Steps taken to test:

  1. Created several articles and book pages
  2. Added articles/pages to new book with different weights and parentage
  3. Viewed book, checked if CSS loaded by:
    • Inspecting individual elements in the Elements tab and checking their CSS inheritance
    • using jQuery to find known selectors from the theme
  4. Everything checks out, the book.theme.css loads successfully.

However, when I go to edit a book page (but NOT an article) or view the outline, the following error appears above the edit box:

Notice: Trying to get property of non-object in Drupal\menu_link\MenuLinkStorageController->findChildrenRelativeDepth() (line 567 of core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php).
Notice: Trying to get property of non-object in Drupal\menu_link\MenuLinkStorageController->findChildrenRelativeDepth() (line 573 of core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php).
Notice: Trying to get property of non-object in Drupal\menu_link\MenuLinkStorageController->findChildrenRelativeDepth() (line 580 of core/modules/menu_link/lib/Drupal/menu_link/MenuLinkStorageController.php).

Thank for the thorough review!

I don't think those notices have anything to do with this patch (might even be related to #2012920: Add child page link in book navigation does not insert parent into book outline), but will check them out.

Status:Reviewed & tested by the community» Needs review

So can someone reproduce the problem without the patch?

Re-rolled patch to keep up with HEAD.

re: #5 and #7

I think it's something to do with caching.

Can you try this:
1) Pull latest changes.
2) Apply patch
3) Do a clean install (will need to remove sites/default/files/php and sites/default/files/config_*, first).
4) Test creating a book.

I've just done this and have no errors.

StatusFileSize
new1012 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,278 pass(es).
[ View ]

Did that forgetting to upload the patch, thing…

Yes, tested on clean copy of 8.x, aforementioned error exists irrespective of this patch.

Pulled, applied, wiped sites/default/files etc, tested as above.

Some CSS appears to have loaded, but book.theme.css throws a 404 error when selected in the resources tab and the elements report that they're drawing CSS from themes/bartik, which does not include the subelement selectors like "next" and "up."

Either needs re-rolling or it's me?

StatusFileSize
new1016 bytes
PASSED: [[SimpleTest]]: [MySQL] 55,404 pass(es).
[ View ]

Ah, the 404 is explained by http://drupalcode.org/project/drupal.git/commit/2432c02 which moved the CSS.
Rerolled!

Status:Needs review» Reviewed & tested by the community

Ok, that appears to work, book.theme.css loads, elements are displayed accordingly, tested with several content types. Ditto above, but additionally: success!

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Status:Fixed» Closed (fixed)

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