Attaching scripts to blocks using the current implementation will fail if block cache is turned on since the hook_block_view_alter will never be called.

This patch checks the cache setting of the block, and if cacheable adds a small script loader to the block's markup so it'll always be in the cached version. Because it's loaded via an HTTP request, it should have a similar effect to putting it in the footer as it won't block the page as inline js would.

CommentFileSizeAuthor
#5 1404206-5.inline-cached-blocks.patch2.48 KBksenzee
#4 1404206-4.inline-cached-blocks.patch2.48 KBksenzee
#3 js-libs-inline-cached-blocks-3.patch2.09 KBmsonnabaum
js-libs-inline-cached-blocks.patch1.83 KBmsonnabaum

Comments

pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work

I don't think that's the right approach - what if JS aggregation is enabled?

msonnabaum’s picture

msonnabaum commented

Well, clearly it won't be aggregated. That seems like a reasonable trade off unless I'm missing some other consequence of this.

msonnabaum’s picture

msonnabaum commented
Status: Needs work » Needs review
StatusFileSize
new js-libs-inline-cached-blocks-3.patch2.09 KB

Here's a new version that makes sure block cache is enabled before doing anything, and it also checks Drupal.settings before adding a script to make sure it hasn't been added in another block already.

ksenzee’s picture

ksenzee
she/her
commented
StatusFileSize
new 1404206-4.inline-cached-blocks.patch2.48 KB

Talked to pwolanin and msonnabaum about this, and we agreed it's more consistent to use the script loader for all block JS, regardless of whether the block is currently cached or not, to avoid changing the order of script loading when block caching gets turned on. Here's a new version that

- always uses the script loader for block JS
- resolves stream wrappers, so jQuery isn't trying to GET public://some.js
- makes some fixes to the script loader JS

ksenzee’s picture

ksenzee
she/her
commented
StatusFileSize
new 1404206-5.inline-cached-blocks.patch2.48 KB

Oops. Stray single quote at the end of the script block, fixed in the attached patch.

pwolanin’s picture

pwolanin commented

Do we even need this patch, if this patch goes in to core: https://drupal.org/node/1460766 ?

pwolanin’s picture

pwolanin commented
Status: Needs review » Fixed

committed

Status: Fixed » Closed (fixed)

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