Problem/Motivation

Once upon a time we used to call drupal_flush_all_caches() at the end of the installer. This resulted in _drupal_flush_css_js() being called that sets the dummy query string added to all CSS and JavaScript files.

Potentially if you browser has cached a older version of the CSS because the query string is not during an install you could end up with an old dated version of a CSS or JS file.

Also some tests expect this to be set because we (unnecessarily) call drupal_flush_all_caches() during BrowserTestBase site installation - see #2189411: Remove an unnecessary container rebuild from FunctionalTestSetupTrait

Steps to reproduce

Install Drupal - and you'll see hrefs like /core/assets/vendor/normalize-css/normalize.css?0. If you flush caches then these will change to something like /core/assets/vendor/normalize-css/normalize.css?qr9lk0.

Proposed resolution

Ensure that the state value is set by the installer.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

CommentFileSizeAuthor
#12 3207893-12.patch1.96 KBSpokje
#12 interdiff_2_12.txt676 bytesSpokje
#2 3207893-2.patch1.19 KBalexpott
#2 3207893-2-test-only.patch788 bytesalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review
FileSize
3207893-2-test-only.patch788 bytes
3207893-2.patch1.19 KB

The last submitted patch, 2: 3207893-2-test-only.patch, failed testing. View results

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Bug Smash Initiative

Looks good to me

  • catch committed f0f758a on 9.2.x 
    Issue #3207893 by alexpott: Set system.css_js_query_string during...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!

  • catch committed fd11f57 on 9.1.x 
    Issue #3207893 by alexpott: Set system.css_js_query_string during...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

We had a race here between #2189411: Remove an unnecessary container rebuild from FunctionalTestSetupTrait and this.
It looks like #2189411: Remove an unnecessary container rebuild from FunctionalTestSetupTrait got in first, so we can probably remove the extra call to _drupal_flush_css_js in FunctionalTestSetupTrait as well as the @todo that links here - follow up or should we just reuse this issue?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Fixed » Needs work

I've rolled back the commit from this issue so we can commit the single change here - will be easier to track in git history later on.

  • catch committed 500ba2d on 9.2.x 
    Revert "Issue #3207893 by alexpott: Set system.css_js_query_string...

  • catch committed ab59d3b on 9.1.x 
    Revert "Issue #3207893 by alexpott: Set system.css_js_query_string...
Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje commented
Status: Needs work » Needs review
FileSize
interdiff_2_12.txt676 bytes
3207893-12.patch1.96 KB
Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje commented

We had a race here between #2189411: Remove an unnecessary container rebuild from FunctionalTestSetupTrait and this.

Let's see if this ends the race.
* prepares to wave chequered flag *

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje commented

* prepares to wave chequered flag *

* Drops flag on floor, checks and sees that #2189411: Remove an unnecessary container rebuild from FunctionalTestSetupTrait never made it into 9.1.x *.

So for:

9.1.x: Patch #2
9.2.x: Patch#12

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje commented
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Looks good to me!

Spokje’s picture

Spokje
Primary language Dutch
CreditAttribution: Spokje commented
Version: 9.1.x-dev » 9.2.x-dev

Changing version to 9.2.x-dev in the hope that will trigger a two daily test rerun of patch #12.

Edit: No dice :/

  • catch committed 9db502c on 9.2.x 
    Issue #3207893 by alexpott, Spokje, larowlan, catch: Set system....

  • catch committed eae19a5 on 9.1.x 
    Issue #3207893 by alexpott, Spokje, larowlan, catch: Set system....
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

OK, committed the respective patches to the respective branches, thanks everyone!

Status: Fixed » Closed (fixed)

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