Page MenuHomePhabricator

Clarify (establish) CSS attribute selector guideline
Closed, ResolvedPublic

Description

Our CSS Coding conventions don't include the a clear guideline on quoting or not quoting attribute selectors.
There is an extensive article on when it is valid to unquote attribute selectors by Mathias Bynens:

a valid unquoted attribute value in CSS is any string of text that is not the empty string, is not just a hyphen (-), consists of escaped characters and/or characters matching /[-_\u00A0-\u10FFFF]/ entirely, and doesn’t start with a digit or two hyphens or a hyphen followed by a digit.

Two questions:

  1. Quote or unquote?
    1. Which quote – single or double?

It would either make sense to consistently quote all attribute selectors (with one type of quote) and have CSS Minifier delete the syntactical sugar where applicable or unqote per default where applicable (majority of the cases apart from content.externallinks.css which doesn't seem to be in use any more. If we decide pro quoting we should also revisit guidelines on url( image.png ) values there


Further reading
JavaScript coding guidelines pledge for single quotes on string literals, although it doesn't clarify on HTML constructs like

$content.find( 'input[type="checkbox"]:not(.noshiftselect)' ).checkboxShiftClick();

Src: MW core

var $checkboxes = $( 'li input[type=checkbox]' );

Src: MW core

JavaScript coding conventions do include a specific sentence on CSS attribute selectors pitfalls in jQuery, though linking to an invalid jQuery bug, where the author featured an invalid selector:

Consistently quote attribute selector values: [foo="bar"] instead of [foo=bar] (jqbug 8229).

Event Timeline

JavaScript coding conventions do include a specific sentence on CSS attribute selectors pitfalls in jQuery, though linking to an invalid jQuery bug, where the author featured an invalid selector:

Consistently quote attribute selector values: [foo="bar"] instead of [foo=bar] (jqbug 8229).

jquery bug #8229 is invalid because the author didn't use quotes. The bug also shows how this behaviour for unquoted values changed without warning or deprecation - in a minor release.

That bug was about jQuery v1.5 (2011). Apparently it happened again more recently in jQuery v1.12 (2015) when it was aligned with the querySelector specification (see https://github.com/jquery/jquery/issues/2824).

More importantly, the bug quotes https://api.jquery.com/attribute-equals-selector/ stating that "quotes are mandatory", meaning any other use may or may not work at one's own risk. More recently, jQuery documentation has been loosened a little bit. Now stating "Can be either an unquoted single word or a quoted string". See also https://api.jquery.com/category/selectors/attribute-selectors/.

Back to CSS guidelines, I would recommend we standardise on always quoting attributes in CSS selectors. It's a small cost to pay for a big win in consistency. It completely removes any doubt about escaping rules and exceptions for when one can't use an quoted value. Ease for writing CSS, ease for reviewing code. It also makes it consistent with authored HTML.

I'd use double quotes since that's what we use in general for HTML. Both when authored in PHP and in JavaScript (since we use single quotes for string literals in both languages). Current code shows that the majority of our CSS already uses double quotes.

Krinkle triaged this task as Medium priority.Apr 8 2016, 7:54 PM

This task has been assigned to the same task owner for more than two years. Resetting task assignee due to inactivity, to decrease task cookie-licking and to get a slightly more realistic overview of plans. Please feel free to assign this task to yourself again if you still realistically work or plan to work on this task - it would be welcome!

For tips how to manage individual work in Phabricator (noisy notifications, lists of task, etc.), see https://phabricator.wikimedia.org/T228575#6237124 for available options.
(For the records, two emails were sent to assignee addresses before resetting assignees. See T228575 for more info and for potential feedback. Thanks!)

Krinkle claimed this task.

This has been covered by https://github.com/wikimedia/stylelint-config-wikimedia for a while.
Future changes should be proposed in that repo instead.

I've updated https://www.mediawiki.org/wiki/Manual:Coding_conventions/CSS accordingly.

  NODES
Coding 5
see 5