Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Was noticing an excessive number of calls to drupal_add_css() and discovered that the logic in views_plugin_cache::gather_headers() is a bit broken. The array it's checking against to get the diffs is cleared right before it, which results in the entire $css and $js array being cached.
Attached patch simplifies the logic and appears to work well in my testing.
Comment | File | Size | Author |
---|---|---|---|
#4 | 1530740-cache-storage.patch | 5.77 KB | dawehner |
#2 | 1530740-test-cache-storage.patch | 3.47 KB | dawehner |
#2 | 1530740-cache-storage.patch | 5.18 KB | dawehner |
views_fix_cssjs_cache_add.patch | 1.71 KB | msonnabaum | |
Comments
Comment #1
gregglesApplies cleanly and seems to reduce the time spent in drupal_add_css in brief xhprof testing.
Let's see what testbot thinks of it.
Comment #2
dawehnerHere is a test for this issue. There is one expected test fail.
Comment #3
tim.plunkettOverall this looks great, and it's wonderful to see a test!
Coding/grammar nitpicks:
Use @todo, and no :, and a trailing full stop. Unless this is to be fixed before committing?
s/Take sure/Make sure
s/Take sure/Make sure
Missing trailing full stop.
Use @todo, and no :, and a trailing full stop. Unless this is to be fixed before committing?
s/Take sure/Make sure
s/Take sure/Make sure
Comment #4
dawehnerdawehner-- for doing the same stupid thing over and over again.
I'm confused you posted the same review twice, how does dreditor allow you to do that.
Comment #5
dawehnerLet's get it in, the tests pass and the code itself looks fine.
This maybe is worth to backport.
Comment #6
Chris Matthews CreditAttribution: Chris Matthews as a volunteer commentedThe Drupal 6 branch is no longer supported, please check with the D6LTS project if you need further support. For more information as to why this issue was closed, please see issue #3030347: Plan to clean process issue queue