Page MenuHomePhabricator

ResourceLoader mangles @import urls with query strings to remove query string
Closed, ResolvedPublic

Description

https://bits.wikimedia.org/it.wikivoyage.org/load.php?debug=true&lang=it&modules=site&only=styles&skin=vector returns, as of 1.23wmf6:

/* MediaWiki:Common.css */
/* Gli stili CSS inseriti qui si applicano a tutte le skin */

/* Formattazione del template QuickBar */
@import url('//it.wikivoyage.org/w/index.php?title=MediaWiki:Quickbar.css&action=raw&ctype=text/css');

/* Multiline tables */
@import url(//it.wikivoyage.org/w/index.php);

/* Stili del template Babel */
@import url(//it.wikivoyage.org/w/index.php);

/* Edittools: Specialchars */
@import url(//it.wikivoyage.org/w/index.php);

etc. etc. All @import URLs except the first one are getting their query string stripped.

The most obvious suspect is https://gerrit.wikimedia.org/r/94511


Version: unspecified
Severity: normal

Details

Reference
bz58338

Event Timeline

bzimport raised the priority of this task from to Unbreak Now!.Nov 22 2014, 2:43 AM
bzimport set Reference to bz58338.

There are two bugs hidden here. My patch introduced neither, but exposed them both by making the regex used to detect 'url()' values more lax and thus matching the @imports that are the concern here.

The first bug is that protocol-relative URLs are not being detected and were processed as if they were paths to files.

The second bug is that even if an URL is detected, the query part is dropped.

The only reason this hasn't come up before is because the 'url()' values are used almost exclusively for background-images (and related styles), and almost all such images are pulled from Commons, which does not require any query parameters to be provided.

I'm writing a patch to fix both of these problems.

The fact that '@import url()' rule is now being considered for remapping might be an issue in theory (I honestly don't know if we should remap them), but should not be an issue in practice once we fix the two bugs above.

Change 100824 had a related patch set (by Bartosz Dziewoński) published:
CSSMin: Fix remapOne() for URLs that are proto-relative or have query part

https://gerrit.wikimedia.org/r/100824

I don't if the patch has been already implemented or not, but the effect of the bug still persist.

To verify that everything works you can check:

  1. the main page: https://it.wikivoyage.org/wiki/Pagina_principale
  2. One generic article with banner: e.g. https://it.wikivoyage.org/wiki/Firenze

Change 100918 had a related patch set uploaded by Jforrester:
CSSMin: Fix remapOne() for URLs that are proto-relative or have query part

https://gerrit.wikimedia.org/r/100918

Change 100824 merged by jenkins-bot:
CSSMin: Fix remapOne() for URLs that are proto-relative or have query part

https://gerrit.wikimedia.org/r/100824

Andy, the bug has been fixed, but the fix has not yet been deployed. I think James and Roan committed themselves to getting it deployed today.

  • Bug 58358 has been marked as a duplicate of this bug. ***

Change 100918 merged by Catrope:
CSSMin: Fix remapOne() for URLs that are proto-relative or have query part

https://gerrit.wikimedia.org/r/100918