Recommend commit message:

Issue #1934508 by jwilson3, clemens.tolboom, wesleydv, matglas86, longwave, sidharthap, superspring, MegaChriz: Make cache clear affect logo and favicon.

Problem/Motivation

Drupal has a great piece of functionality for front-end developers to force browsers to reload css and javascript. However, this same courtesy is not extended to the favicon and the logo image.

Developers using a git/svn based deployment process cannot overwrite the logo.png or favicon.ico files in a theme, deploy into a production environment, and expect the site to display the updated files because browsers that have already visited the site will pull these files from their cache.

From a front-end developer perspective, theme_get_setting does not provide any clean method to alter the data in order to add a cache-busting query string from a custom module or theme, thus the only way to ensure all browsers get a new version of these files is to either specify a separate path to a different logo or favicon file or upload a file with a different name. This can be done only via the Appearance page in the Drupal administrative user interface or by writing an update script in a custom module.

Proposed resolution

The proposed solution here appends the standard Drupal browser cache-busting string (css_js_query_string) to the logo and favicon URLs generated in theme_get_setting.

Remaining tasks

None.

User interface changes

No visible changes out of the box. But, because we've added query strings to the logo and to the favicon URLs, a net positive effect on rendering will occur when a site administrator updates the favicon or logo files on the server. A subsequent cache clear will guarantee all visitors will see the new file instead of the old file from cache.

API changes

The Drupal CSS/JS query-string internal variable has been renamed from css_js_query_string to drupal_asset_query_string, and the internal function _drupal_flush_css_js() to _drupal_flush_assets(), because both affect not only CSS and JS anymore.

CommentFileSizeAuthor
#91 interdiff.txt735 bytesjoelpittet
#91 cache_clear_doesn_t-1934508-91.patch14.98 KBjoelpittet
#89 interdiff-kinda.txt20.06 KBjoelpittet
#89 cache_clear_doesn_t-1934508-89.patch14.97 KBjoelpittet
#83 cache_clear_doesn_t-1934508-82.patch14.95 KBclemens.tolboom
#80 cache_clear_doesn_t-1934508-80.patch14.62 KBclemens.tolboom
#77 cache_clear_doesn_t-1934508-77.patch14.87 KBclemens.tolboom
#77 interdiff-1934508-72-77.txt699 bytesclemens.tolboom
#72 interdiff-1934508-71-72.txt5.38 KBclemens.tolboom
#72 cache_clear_doesn_t-1934508-72.patch14.83 KBclemens.tolboom
#71 cache_clear_favicon-1934508-71.patch14.69 KBjwilson3
#70 cache_clear_favicon-1934508-70.patch31.92 KBjwilson3
#66 1934508-cache-clear-logo-66.patch13.78 KBjwilson3
#66 interdiff.txt1007 bytesjwilson3
#65 1934508-cache-clear-logo-65.patch13.78 KBjwilson3
#65 interdiff.txt9.19 KBjwilson3
#60 interdiff-1934508.txt1.3 KBlongwave
#60 1934508-cache-clear-logo-60.patch5.24 KBlongwave
#56 interdiff.txt1.11 KBjwilson3
#55 clear_cache_logo_favicon-1934508-55.patch5.23 KBjwilson3
#51 clear_cache_logo_favicon-1934508-51.patch5.27 KBsidharthap
#3 1934508-cache-controlled-logo-and-favicon-d7-do-not-test.patch2.66 KBjwilson3
#42 clear_cache_logo_favicon-1934508-42.patch5.26 KBsuperspring
#38 1934508-favicon-38.patch5.24 KBjwilson3
#38 1934508-37-38-interdiff.txt3.26 KBjwilson3
#37 1934508-favicon-37.patch4.21 KBwesleydv
#37 1934508-favicon-37-interdiff.txt644 byteswesleydv
#35 1934508-favicon-35.patch4.21 KBwesleydv
#35 1934508-favicon-35-interdiff.txt586 byteswesleydv
#33 1934508-favicon-33.patch4.2 KBwesleydv
#33 1934508-favicon-30-interdiff.txt573 byteswesleydv
#30 1934508-favicon-30.patch4.19 KBwesleydv
#23 1934508-favicon-23.patch5.12 KBmatglas86
#23 1934508-favicon-23-interdiff.txt0 bytesmatglas86
#19 1934508-favicon-19.patch4.94 KBmatglas86
#19 1934508-favicon-19-interdiff.txt639 bytesmatglas86
#9 cache-clear-doesnt-affect-logo-1934508-interdiff-2-7.txt2.46 KBclemens.tolboom
#7 drupal-cache-controlled-logo-and-favicon-1934508-7.patch4.93 KBMegaChriz
#3 1934508-cache-controlled-logo-and-favicon-d7-do-not-test.patch2.66 KBjwilson3
#2 1934508-cache-controlled-logo-and-favicon-2.patch2.66 KBjwilson3
#1 1934508-cache-controlled-logo-and-favicon.patch2.66 KBjwilson3
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jwilson3’s picture

Status: Active » Needs review
FileSize
2.66 KB
jwilson3’s picture

+++ b/core/includes/theme.incundefined
@@ -1436,32 +1436,48 @@ function theme_get_setting($setting_name, $theme = NULL) {
+          $cache[$theme]['logo'] .= $logo_path . $query_string_separator . $query_string;

this should be an assignment operator not a concatenation.

fixed in this patch.

jwilson3’s picture

Status: Needs review » Needs work

The last submitted patch, 1934508-cache-controlled-logo-and-favicon-2.patch, failed testing.

richardj’s picture

The test seems to make no different between 'filename.png' and 'filename.png?abc' which is the whole issue here, to let the browser know that there is a difference between those files so the browsers will not use their agressive browser caching on the favicon.

richardj’s picture

patch in #2 works locally and as intended. See #5 for why test may fail.

MegaChriz’s picture

Status: Needs work » Needs review
FileSize
4.93 KB

In addition of the patch in #2, this patch just simply patches the Drupal\system\Tests\System\ThemeTest and adds the query string to the expected image urls in the test.

jwilson3’s picture

Thanks for following up with the tests! I saw the failures but didnt get a chance to fix them myself earlier this week. Lets see how it turns out... (still yellow).

clemens.tolboom’s picture

Added interdiff-2-7

I am not sure how to test this. The logo path http://drupal.d8/sites/default/files/logo.png?mjg1v7 looks ok.

richardj’s picture

You can test the first patch by applying it, clear cache and open up the html source and look for the favicon.ico and logo.png.

clemens.tolboom’s picture

@richardj sorry I was not clear enough.

I've tested the logo right? But I'm not a themer so am not sure how to test this exactly. Browser quirks etc.

The tests pass and the code looks good to me. So as for me it's RTBC :)

richardj’s picture

Status: Needs review » Reviewed & tested by the community

@clemens.tolboom There are no browser quirks involved in this issue, because the essential image doesn't change, what is done here is essentially what happens with CSS and JS files (when you check the source you see that they also use query strings to indicate a certain 'version'). So the output to the browser / user stays the same. I hope that clears things up a bit.

It looks good to me as well, so RTBC!

xjm’s picture

xjm’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/includes/theme.incundefined
@@ -1437,32 +1437,48 @@ function theme_get_setting($setting_name, $theme = NULL) {
+      // A dummy query-string is added to filenames, to gain control over
+      // browser-caching. The string changes on every update or full cache
+      // flush, forcing browsers to load a new copy of the files, as the
+      // URL changed.
+      $query_string = variable_get('css_js_query_string', '0');

+++ b/core/modules/system/lib/Drupal/system/Tests/System/ThemeTest.phpundefined
@@ -48,31 +48,34 @@ function testThemeSettings() {
+    // Logo's and favicons receive a query string.
+    $query_string = variable_get('css_js_query_string', '0');

We should use config() instead of variable_get(), since the latter is deprecated.

Also -- I might be mistaken, but isn't this an optional configuration? If so, we might want to re-think the way we're changing the tests here. (I could be wrong.)

xjm’s picture

Category: bug » feature
Priority: Minor » Normal

Also, while this is a great change, I think it's more of a feature request.

richardj’s picture

I think this can be considered a feature, but it is something like caching / compressing css and javascript, quite essential.

This is something small that doesn't really benefit from having a on / off switch, but it benefits from being at least there.

xjm’s picture

jwilson3’s picture

Agree with #16, to grant these two elements their own on/off cache control switches on the theme general settings (or in the admin performance page) seems overkill -- this is a tiny "silent" feature for themers that should just work. I suppose there could be a variable that if set in your settings.php file, would specifically exclude the query string from the favicon and icon, if you so desire, but really, whats the point?

matglas86’s picture

I replace the variable_get with the state()-get() as suggested in #17.

matglas86’s picture

Status: Needs work » Needs review

Ooh I forgot to set it to needs review.

Status: Needs review » Needs work

The last submitted patch, 1934508-favicon-19.patch, failed testing.

xjm’s picture

Agree with #16, to grant these two elements their own on/off cache control switches on the theme general settings (or in the admin performance page) seems overkill -- this is a tiny "silent" feature for themers that should just work. I suppose there could be a variable that if set in your settings.php file, would specifically exclude the query string from the favicon and icon, if you so desire, but really, whats the point?

Fair enough. Thanks!

matglas86’s picture

Status: Needs work » Needs review
FileSize
0 bytes
5.12 KB

I figured that there is a problem with in the install process and getting state(). So this is no surrounded with try catch. That was the result of the test. It should be working now.

MathieuSpil’s picture

Issue tags: -Needs themer review

#23: 1934508-favicon-23.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs themer review

The last submitted patch, 1934508-favicon-23.patch, failed testing.

jwilson3’s picture

Edit: Removed useless rant ;O

jwilson3’s picture

jwilson3’s picture

Status: Needs work » Needs review
jwilson3’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

patch in #23 needs a reroll.

wesleydv’s picture

FileSize
4.19 KB

Reroll against head

wesleydv’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll

Status: Needs review » Needs work

The last submitted patch, 1934508-favicon-30.patch, failed testing.

wesleydv’s picture

Status: Needs work » Needs review
FileSize
573 bytes
4.2 KB

Fixing issue from patch #30

Status: Needs review » Needs work

The last submitted patch, 1934508-favicon-33.patch, failed testing.

wesleydv’s picture

Status: Needs work » Needs review
FileSize
586 bytes
4.21 KB

Fixing issue in ThemeTest.php

jwilson3’s picture

Status: Needs review » Needs work

Sorry for this nitpick, but:

+++ b/core/modules/system/lib/Drupal/system/Tests/System/ThemeTest.php
@@ -49,31 +49,39 @@ function testThemeSettings() {
+    // Logo's and favicons receive a query string.

Incorrect use of apostrophe. Looks like this got introduced way back in #7, sorry I didn't catch this earlier.

Otherwise, it looks great to me.

wesleydv’s picture

Status: Needs work » Needs review
FileSize
644 bytes
4.21 KB

Fixed typo described in #36

jwilson3’s picture

Thanks for that fix.

  1. +++ b/core/includes/theme.inc
    @@ -1422,6 +1434,10 @@ function theme_get_setting($setting_name, $theme = NULL) {
    +        if (!empty($favicon_path)) {
    +          $query_string_separator = (strpos($favicon_path, '?') !== FALSE) ? '&' : '?';
    +          $cache[$theme]['favicon'] = $favicon_path . $query_string_separator . $query_string;
    +        }
    

    This is broken code $cache[$theme]['favicon'] if this code is ever reached, it won't work and will cause a fatal error "Fatal error: Cannot use object of type Drupal\Core\Theme\ThemeSettings as array".

  2. +++ b/core/modules/system/lib/Drupal/system/Tests/System/ThemeTest.php
    @@ -49,31 +49,39 @@ function testThemeSettings() {
    +    // Logos and favicons receive a query string.
    

    Code comments should use the hyphenated text "query-string".

  3. From a manual test of #37 using spark theme, I can see that neither the default favicon nor the default logo image receive the query-string, and only images that are uploaded via the UI get the query string. This defeats the entire reason for filing this issue, which is to serve the folks who want to update their theme's default logo.png and favicon.ico files (inside their theme folder) without needing to upload new files via Drupal's admin UI. The idea is you update your theme asset files in the file system via FTP, version control, or other code deployment mechanism, then clear the Drupal cache, and Done.

I've refactored the code to address all three of these issues, since there is a bit of structural flow in the if statements, its probably best to apply the patch and read through it, instead of trying to figure out whats going on just reading the patch.

jwilson3’s picture

+++ b/core/includes/theme.inc
@@ -1395,30 +1395,47 @@ function theme_get_setting($setting_name, $theme = NULL) {
+          if (!file_exists($logo_path = dirname($theme_object->filename) . '/logo.png')) {
+            $logo_path = FALSE;
+          }
...
+          if (!file_exists($favicon_path = dirname($theme_object->filename) . '/favicon.ico')) {
+            $favicon_path = 'core/misc/favicon.ico';
           }

If the theme is using default favicon and a favicon.ico file is not provided in the theme, then it uses Drupal's core favicon.

However, if the theme is using the default logo and logo.png doesn't exist in the theme, then it displays an img tag with an empty src attribute.

This should be consistent.

We should make the default logo_path be 'core/misc/druplicon.png' if no logo is found in the theme.

jwilson3’s picture

Status: Needs review » Needs work
jwilson3’s picture

Issue tags: -Needs themer review +Novice, +php-novice

Tagging in case anyone wants to grab this and roll in #39.

superspring’s picture

Status: Needs work » Needs review
FileSize
5.26 KB

This is taking the patch from #38 and applying the review in #39

superspring’s picture

Issue summary: View changes

Clarified first paragraph

rodrigoaguilera’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

Tested that it has the correct behaviour and implements the last remarks

jwilson3’s picture

Issue summary: View changes

I've cleaned up the issue summary.

jwilson3’s picture

Issue summary: View changes
catch’s picture

Status: Reviewed & tested by the community » Needs work

This looks like a good change, but if we do this I think we should also rename the query string to drupal_asset_query_string or similar.

drupalninja99’s picture

@catch do you mean replace $query_string with $drupal_asset_query_string and $query_string_separator with $drupal_asset_query_string_separator?

sidharthap’s picture

$query_string = variable_get('css_js_query_string', '0');
should we use config here instead of variable_get?

MegaChriz’s picture

@sidharthap
Yes, config should be used instead of variable_get()
See also: https://drupal.org/node/2183531

catch’s picture

I mean changing the key used in state.

+++ b/core/includes/theme.inc
@@ -1395,30 +1395,47 @@ function theme_get_setting($setting_name, $theme = NULL) {
+        $query_string = \Drupal::state()->get('system.css_js_query_string') ?: '0';

Like here.

sidharthap’s picture

Status: Needs work » Needs review
FileSize
5.27 KB

Thanks @catch
Corrected the $query_string key.

jwilson3’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Novice, -php-novice

The change between #42 and #51 looks fine.

jwilson3’s picture

Issue summary: View changes

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 51: clear_cache_logo_favicon-1934508-51.patch, failed testing.

jwilson3’s picture

This needed a re-roll, due to #2210197: Remove public access to Extension::$type, ::$name, ::$filename.

Which requires us to convert from dirname($theme_object->filename) to $theme_object->getPath().

jwilson3’s picture

FileSize
1.11 KB

Forgot to attach an interdiff between 51 and 55, here it is.

longwave’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 55: clear_cache_logo_favicon-1934508-55.patch, failed testing.

jwilson3’s picture

I don't understand enough about core testing procedures to grok why there were so many seemingly unrelated fails. Anyone have any ideas?

longwave’s picture

Status: Needs work » Needs review
FileSize
5.24 KB
1.3 KB

$theme_object->getPath should be $theme_object->getPath(), plus I fixed a minor coding standards issue.

jwilson3’s picture

Bah. thank you! AND sorry :-|

jwilson3’s picture

Status: Needs review » Reviewed & tested by the community

This looks good, patch results are green, and it contains the feedback from #46, back to RTBC.

jwilson3’s picture

Issue summary: View changes
alexpott’s picture

Status: Reviewed & tested by the community » Needs work

@catch so

+++ b/core/includes/theme.inc
@@ -887,30 +887,47 @@ function theme_get_setting($setting_name, $theme = NULL) {
+        $query_string = \Drupal::state()->get('system.drupal_asset_query_string') ?: '0';

So this string is not altered on cache clear so this change will not work. We should be testing this.

I guess that @catch meant rename all instances of system.css_js_query_string

jwilson3’s picture

Status: Needs work » Needs review
FileSize
9.19 KB
13.78 KB

I've renamed system.drupal_asset_query_string everywhere and added a test for _drupal_flush_css_js() to catch the case mentioned in #64. Presumably we should *not* rename the function to _drupal_flush_assets(), but perhaps it does make sense to update the docblock for that function to mention logo and favicon will also be affected, in addition to css and js. Thoughts?

jwilson3’s picture

+++ b/core/modules/system/src/Tests/System/ThemeTest.php
@@ -177,7 +185,40 @@ function testThemeSettings() {
+    // Ensure that a cache flush reset the query-string from its original value.

grammar :( fixed in this follow up.

The last submitted patch, 65: 1934508-cache-clear-logo-65.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 66: 1934508-cache-clear-logo-66.patch, failed testing.

jwilson3’s picture

There is something wrong with the test I added, but I don't know what it is. Need a second set of eyeballs on this.

jwilson3’s picture

Status: Needs work » Needs review
FileSize
31.92 KB

I was finally able to figure out that problem above boiled down to the use of REQUEST_TIME for calculating the query string. During a test run, a non-existent query string is first created and then reset within the context of one single request, which was having the effect of recalculating the query string based on the same REQUEST_TIME, hence the query string did not seem to change,.

The solution was to base the query string on the current unix timestamp time() instead of REQUEST_TIME. This solved the problem, and tests now pass (locally for me). Lets see what test bot says...

jwilson3’s picture

Heh. I had commented out other tests to speed up the testing process. Here they are back to normal.

clemens.tolboom’s picture

The try-catch is weird and was not documented.

It was found through #23 @matglas86 ... is that still needed?

As StateInterface::get() has a default I removed the ?: construct.

Status: Needs review » Needs work

The last submitted patch, 72: cache_clear_doesn_t-1934508-72.patch, failed testing.

clemens.tolboom’s picture

Running the failure on #72 I ran into #2306649: Running phpunit tests via the UI results in a fatal when running the failing test locally.

Applying the patch from #2306649-1: Running phpunit tests via the UI results in a fatal made Drupal\Tests\Core\Asset\CssCollectionRendererUnitTest run like a charm locally.

So not sure why it failed.

The last submitted patch, 72: cache_clear_doesn_t-1934508-72.patch, failed testing.

clemens.tolboom’s picture

Status: Needs work » Needs review
FileSize
699 bytes
14.87 KB

Hmmm ... the fix is changing the mock returnung '0' instead of 'NULL'.

Running the test through the Simpletest UI changes the conditions as next check behaves differently as running through Simpletest make the core function available instead.

if (!function_exists('file_create_url')) {

Hope I understood this correctly.

Status: Needs review » Needs work

The last submitted patch, 77: cache_clear_doesn_t-1934508-77.patch, failed testing.

clemens.tolboom’s picture

Status: Needs work » Needs review
FileSize
14.62 KB

Patch needed a rebase. Thanks for the reminder.

Status: Needs review » Needs work

The last submitted patch, 80: cache_clear_doesn_t-1934508-80.patch, failed testing.

clemens.tolboom’s picture

Status: Needs work » Needs review

I ran
sudo -u _www php run-tests.sh --url http://drupal.d8 --class "Drupal\system\Tests\Common\JavaScriptTest,Drupal\Tests\Core\Asset\CssCollectionRendererUnitTest,Drupal\system\Tests\Common\CascadingStylesheetsTest,Drupal\system\Tests\System\ThemeTest"
which failed on the last. There was a missed reroll _drupal_flush_css_js(); which should have been _drupal_flush_assets(); ... weird the testbot is not more informative.

clemens.tolboom’s picture

jwilson3’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, passes tests, and passed manual test on http://s60c825e9d4ac265.s2.simplytest.me/.

Initial install has query-string on the druplicon logo as ?0. Clicking Performance > Clear All Caches, once sets the query string to ?nanc2n on the logo. clearing the caches a second time set the query-string to ?nakc3f.

RTBC.

jwilson3’s picture

Hiding old patches.

jwilson3’s picture

Issue summary: View changes
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/includes/theme.inc
    @@ -922,33 +922,48 @@ function theme_get_setting($setting_name, $theme = NULL) {
    +      // A dummy query-string is added to filenames, to gain control over
    +      // browser-caching. The string changes on every update or full cache
    +      // flush, forcing browsers to load a new copy of the files, as the
    +      // URL changed.
    +      // We need try-catch due to the installer not having a state yet.
    

    Also the comment is not wrapped correctly and the last sentence is not grammatically correct. Maybe We need to catch WhateverException since theme_get_setting() is used during the installer before the state service is available.

  2. +++ b/core/includes/theme.inc
    @@ -922,33 +922,48 @@ function theme_get_setting($setting_name, $theme = NULL) {
    +      try {
    +        $query_string = \Drupal::state()->get('system.drupal_asset_query_string', '0');
    +      }
    +      catch (Exception $e) {
    +        $query_string = '0';
    +      }
    

    What is the specific exception we want to catch here? A general catch seems unwise.

  3. +++ b/core/lib/Drupal/Core/Asset/CssCollectionRenderer.php
    @@ -41,7 +41,7 @@ public function render(array $css_assets) {
    -    $query_string = $this->state->get('system.css_js_query_string') ?: '0';
    +    $query_string = $this->state->get('system.drupal_asset_query_string', '0');
    
    +++ b/core/lib/Drupal/Core/Asset/JsCollectionRenderer.php
    @@ -43,7 +43,7 @@ public function render(array $js_assets) {
    -    $default_query_string = $this->state->get('system.css_js_query_string') ?: '0';
    +    $default_query_string = $this->state->get('system.drupal_asset_query_string', '0');
    
    +++ b/core/modules/system/src/Tests/Common/CascadingStylesheetsTest.php
    @@ -72,7 +72,7 @@ function testRenderFile() {
    -    $query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0';
    +    $query_string = $this->container->get('state')->get('system.drupal_asset_query_string', '0');
    
    @@ -178,7 +178,7 @@ function testAddCssFileWithQueryString() {
    -    $query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0';
    +    $query_string = $this->container->get('state')->get('system.drupal_asset_query_string', '0');
    
    +++ b/core/modules/system/src/Tests/Common/JavaScriptTest.php
    @@ -102,7 +102,7 @@ function testAddExternal() {
    -    $default_query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0';
    +    $default_query_string = $this->container->get('state')->get('system.drupal_asset_query_string') ?: '0';
    
    @@ -129,7 +129,7 @@ function testAggregatedAttributes() {
    -    $default_query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0';
    +    $default_query_string = $this->container->get('state')->get('system.drupal_asset_query_string') ?: '0';
    
    @@ -347,7 +347,7 @@ function testDifferentWeight() {
    -    $default_query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0';
    +    $default_query_string = $this->container->get('state')->get('system.drupal_asset_query_string') ?: '0';
    
    @@ -384,7 +384,7 @@ function testVersionQueryString() {
    -    $default_query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0';
    +    $default_query_string = $this->container->get('state')->get('system.drupal_asset_query_string') ?: '0';
    
    @@ -694,7 +694,7 @@ function testAddJsFileWithQueryString() {
    -    $query_string = $this->container->get('state')->get('system.css_js_query_string') ?: '0';
    +    $query_string = $this->container->get('state')->get('system.drupal_asset_query_string') ?: '0';
    

    Considering we are touching all of these lines let's simplify this code by passing in the default value. This was not available when this was written.

joelpittet’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Needs review
FileSize
14.97 KB
20.06 KB

Triage treasure hunting revival. Bumping to 8.1 as it's a feature request.

Re-rolled and did #88.3. See if this even works, hopefully I did ok on re-roll and didn't lose anything in translation.

Status: Needs review » Needs work

The last submitted patch, 89: cache_clear_doesn_t-1934508-89.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
14.98 KB
735 bytes

Typos in the last patch.

Status: Needs review » Needs work

The last submitted patch, 91: cache_clear_doesn_t-1934508-91.patch, failed testing.

joelpittet’s picture

Both failures are now just ColorTest fails, anybody else want to pick this up?

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.