Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is a sub-task of http://drupal.org/node/1775842 Convert all variables to state and/or config systems.
Comment | File | Size | Author |
---|---|---|---|
#16 | remove_js_version_string.patch | 1.32 KB | catch |
#14 | js_version_settings.patch | 2.07 KB | catch |
#9 | 1824848-drupal_js_version_query_string_to_cmi.9.patch | 1.42 KB | mtift |
#4 | 1824848-drupal_js_version_query_string_to_cmi.4.patch | 1.32 KB | spearhead93 |
#3 | 1824848-drupal_js_version_query_string_to_cmi.2.patch | 1.32 KB | spearhead93 |
Comments
Comment #1
spearhead93 CreditAttribution: spearhead93 commentedMoving this variable to 'system.js_version_query_string'.
Comment #3
spearhead93 CreditAttribution: spearhead93 commentedAdding an upgrade path for 'drupal_js_version_query_string'.
Comment #4
spearhead93 CreditAttribution: spearhead93 commentedMoving this one to needs review.
Comment #6
alexpottThis is postponed on #1798738: Move css_js_query_string to state system and #1809206: KeyValueFactory hard-codes DatabaseStorage as this change needs the keyvalue service to work early on in update.php ie. before the state table actually exists.
Comment #7
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #8
sunNot sure whether this belongs into theme system or base system. But definitely not configuration system. :)
Comment #9
mtiftJust a re-roll
Comment #10
mtiftI'm just curious if this patch will pass
Comment #12
mtiftComment #13
catchComment #14
catchUntested patch but this might be enough.
This never gets set anywhere, it's just a get-out in case there's a query string conflict.
However I'm also having a hard time seeing why this is needed at all - the query string on the file being loaded itself doesn't make any difference to what the js sees on the page. So I'll upload another patch to remove this altogether...
Comment #16
catchHere's the patch to just remove it altogether. Re-titling again.
Comment #17
chx CreditAttribution: chx commentedNote that this only changes the possibility to override the v in $_GET['v'] in case some boneheaded JS library is hardwired to look for that. It's 2013. Don't use boneheaded JS libraries!Edit: Note that this only changes the possibility to override the v in $_GET['v'] for the source URL of the JS, not even the page. Why that needs to be overridable is an absolute mystery. Maybe some truly stupid CDN? Can't imagine a real world scenario.
Comment #18
webchickThe original rationale for this appears to be #721400-59: Order JS files according to weight, don't change filenames for aggregated JS/CSS:
"we can't assume that every JS file/library in existence will gracefully accept ?v=X in its URL and we should allow a way for a site to change that behavior to some other query parameter that won't conflict."
Talked about this more with chx in IRC, and he pointed out that the JS aggregation feature itself already causes "if your JS makes assumptions about the URL the source file comes from, you're screwed." So this seems like it was a bogus use case to begin with. Also ran this past nod_ and he said it was good. Yay for one fewer variable! :)
Committed and pushed to 8.x. Thanks!