This is a sub-task of http://drupal.org/node/1775842 Convert all variables to state and/or config systems.

Files: 
CommentFileSizeAuthor
#16 remove_js_version_string.patch1.32 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 59,320 pass(es).
[ View ]
#14 js_version_settings.patch2.07 KBcatch
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#9 1824848-drupal_js_version_query_string_to_cmi.9.patch1.42 KBmtift
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#4 1824848-drupal_js_version_query_string_to_cmi.4.patch1.32 KBspearhead93
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#3 1824848-drupal_js_version_query_string_to_cmi.2.patch1.32 KBspearhead93
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#1 1824848-drupal_js_version_query_string_to_cmi.1.patch640 bytesspearhead93
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new640 bytes
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Moving this variable to 'system.js_version_query_string'.

Status:Needs review» Needs work

The last submitted patch, 1824848-drupal_js_version_query_string_to_cmi.1.patch, failed testing.

StatusFileSize
new1.32 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Adding an upgrade path for 'drupal_js_version_query_string'.

Status:Needs work» Needs review
StatusFileSize
new1.32 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Moving this one to needs review.

Status:Needs review» Needs work

The last submitted patch, 1824848-drupal_js_version_query_string_to_cmi.4.patch, failed testing.

Status:Needs work» Postponed

This 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.

Title:Convert drupal_js_version_query_string to CMI systemConvert drupal_js_version_query_string to the state system

Component:configuration system» theme system

Not sure whether this belongs into theme system or base system. But definitely not configuration system. :)

StatusFileSize
new1.42 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Just a re-roll

Assigned:spearhead93» Unassigned
Status:Postponed» Needs review

I'm just curious if this patch will pass

Status:Needs review» Needs work

The last submitted patch, 1824848-drupal_js_version_query_string_to_cmi.9.patch, failed testing.

Status:Needs work» Postponed

Priority:Normal» Critical

Title:Convert drupal_js_version_query_string to the state systemConvert drupal_js_version_query_string to settings
Status:Postponed» Needs review
StatusFileSize
new2.07 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Untested 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...

Status:Needs review» Needs work

The last submitted patch, js_version_settings.patch, failed testing.

Title:Convert drupal_js_version_query_string to settingsRemove drupal_js_version_string variable (instead of converting it)
Status:Needs work» Needs review
StatusFileSize
new1.32 KB
PASSED: [[SimpleTest]]: [MySQL] 59,320 pass(es).
[ View ]

Here's the patch to just remove it altogether. Re-titling again.

Status:Needs review» Reviewed & tested by the community

Note 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.

Status:Reviewed & tested by the community» Fixed

The 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!

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