Page MenuHomePhabricator

MediaWiki:Common.css does not allow starting and trailing space inside url()
Closed, ResolvedPublic

Description

If you add

.badge-recommendedarticle {
	list-style-image: url( "https://upload.wikimedia.org/wikipedia/commons/thumb/c/c7/Rekommenderad_grön.svg/9px-Rekommenderad_grön.svg.png" );
}

to MediaWiki:Common.css, it's turned into

.badge-recommendedarticle {
	list-style-image: url("/w/ \"https://upload.wikimedia.org/wikipedia/commons/thumb/c/c7/Rekommenderad_grön.svg/9px-Rekommenderad_grön.svg.png\"");
}

The starting "/w/" causes this to fail. This seems kinda dangerous, as https://www.mediawiki.org/wiki/Manual:Coding_conventions/CSS#Whitespace says

One space after a starting and before an ending parentheses (( and )) in selectors (ex. :not()) and properties (ex. rgba()).

ResourceLoader should either be extended to allow leading and trailing spaces, or the style guidelines at mediawiki.org should be revised to no longer recommend this.

Event Timeline

Nirmos created this task.Mar 10 2018, 2:58 PM
Restricted Application added a project: Performance-Team. · View Herald TranscriptMar 10 2018, 2:58 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Krinkle triaged this task as High priority.Mar 12 2018, 8:18 PM

I've confirmed that using quotes inside url() with spaces around is valid in CSS and works in Chrome and Firefox.

I've also confirmed the described bug with a failing unit test.

Change 419087 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] CSSMin: Fix breaking of quoted urls with outer spacing

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

Krinkle closed this task as Resolved.Apr 20 2018, 7:11 PM

Change 419087 merged by jenkins-bot:
[mediawiki/core@master] CSSMin: Fix breaking of quoted urls with outer spacing

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

Change 428371 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@REL1_31] CSSMin: Fix breaking of quoted urls with outer spacing

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

Change 428371 merged by jenkins-bot:
[mediawiki/core@REL1_31] CSSMin: Fix breaking of quoted urls with outer spacing

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

Nirmos added a comment.EditedApr 23 2018, 5:28 PM

I'm a little bit confused here. This bug is about quoted URLs, and that's also what the commit message in https://gerrit.wikimedia.org/r/428371 says. However, RELEASE-NOTES-1.31 says unquoted URLs. Have you actually fixed two different issues here, or is the wording in the release notes a mistake?

I'm a little bit confused here. This bug is about quoted URLs, and that's also what the commit message in https://gerrit.wikimedia.org/r/428371 says. However, RELEASE-NOTES-1.31 says unquoted URLs. Have you actually fixed two different issues here, or is the wording the release notes a mistake?

Thanks, you're right. The bug was only with quoted URLs.

The way we fixed this problem was by improving the way we capture unquoted URLs. CSSMin detects unquoted, then single-quoted, then double-quoted. Its detection for unquoted urls looked at url( "something" ) and saw that it was a url( followed by something that is not a single quote or double quote, and thus captured it as an unquoted url (including the space and double-quote symbol as part of the url).

Because the code change only altered logic intended for unquoted urls, it confused me when writing the release notes.

Change 428385 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] RELEASE-NOTES: Fix typo in CSSMin release note

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

Change 428387 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@REL1_31] RELEASE-NOTES: Fix typo in CSSMin release note

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

Change 428385 merged by jenkins-bot:
[mediawiki/core@master] RELEASE-NOTES: Fix typo in CSSMin release note

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

Change 428387 merged by jenkins-bot:
[mediawiki/core@REL1_31] RELEASE-NOTES: Fix typo in CSSMin release note

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