Including "--" class prefix in selector list prevents other selectors from matching

tuggyne · Post by **tuggyne** » 2016-02-16, 04:54

Recently I ran into an apparent rendering bug with Pale Moon 26.0.3; a particular site's disabled buttons were not showing up with the appropriate style, and after tracing it back, I found that Firefox applied the style, but Pale Moon did not. A test case follows, sufficient to reproduce the problem.

Code: Select all

<!DOCTYPE html>
<html>
<head>
  <style type="text/css">
  input[type=button].--disabled, input[type=button][disabled] {
    color: red;
  }
  </style>
</head>
<body>
  <input type="button" value="Testing" disabled="disabled"></input>
</body>
</html>

Removing the first selector in the list is enough to allow the other one to match the element appropriately. In fact, even removing a single hyphen is enough. Is this some sort of unfortunate interaction with blank vendor prefix detection?

Post by **New Tobin Paradigm** » 2016-02-16, 20:03

A user came into irc and threw this code at me.. I don't understand the nature of this issue or anything.. The user also did not respond to further questions except to link this thread and that I should give this to Moonchild.

[20160216.1401-07] <hatm> Replace line 1137 nsCSSScanner.cpp
[20160216.1401-15] <hatm> if (IsIdentStart(c2)) {
[20160216.1401-19] <hatm> with
[20160216.1401-23] <hatm> (IsIdentStart(c2) || (c2 == '-' && c3 != '>')) {

Maybe this will help someone formulate a pull request along side a github issue..

IN THE FUTURE: I would request that users don't come onto IRC and throw random code solutions at the irc channel. If you have a possible solution please use proper venues to express them. Such as a thread or a github tracking issue.. or what not.

tuggyne · Post by **tuggyne** » 2016-02-17, 02:24

Matt A Tobin wrote:A user came into irc and threw this code at me.. I don't understand the nature of this issue or anything.. The user also did not respond to further questions except to link this thread and that I should give this to Moonchild.

Hmm. Well, I appreciate that someone else noticed this, but that's perhaps a bit suboptimal.

[20160216.1401-07] <hatm> Replace line 1137 nsCSSScanner.cpp
[20160216.1401-15] <hatm> if (IsIdentStart(c2)) {
[20160216.1401-19] <hatm> with
[20160216.1401-23] <hatm> (IsIdentStart(c2) || (c2 == '-' && c3 != '>')) {
Maybe this will help someone formulate a pull request along side a github issue..

The code looks vaguely plausible, but I don't really know if it's correct. Given that Gecko handles this appropriately, it would seem reasonable enough to compare with the algorithm there.

Post by **Moonchild** » 2016-02-20, 19:33

Double-dashes are indicators for css "variables", you shouldn't use them as class names since they are treated special. Unfortunately it seems this interferes with some particular design choices?

Please create a GitHub issue for this with a clear description and why the proposed code change would solve it. Having a mish-mash IRC+forum without code context is not a workable environment for sane development or development choices. The logic behind code changes must be sound and must at least be run through for possible scenarios to verify that it's all correct and doesn't cause regressions.

tuggyne · Post by **tuggyne** » 2016-02-20, 20:21

Moonchild wrote:Double-dashes are indicators for css "variables", you shouldn't use them as class names since they are treated special.

Since CSS variables are not permitted in selectors, this does not appear to be relevant, syntactically; identifiers starting with two dashes are legal as class names, and only the var(--foo) syntax (which is not legal in selectors or class attributes) actually interpolates anything.

Unfortunately it seems this interferes with some particular design choices?

Specifically, this interferes with certain Stack Exchange pages. I originally reported this as a bug there before discovering it was a web compatibility issue with Pale Moon.

Please create a GitHub issue for this with a clear description and why the proposed code change would solve it. Having a mish-mash IRC+forum without code context is not a workable environment for sane development or development choices. The logic behind code changes must be sound and must at least be run through for possible scenarios to verify that it's all correct and doesn't cause regressions.

For clarity: I know nothing about the proposed code change, nor about whoever proposed it. GitHub issue opened, though.

Post by **Moonchild** » 2016-02-20, 20:34

I gather the scanner code simply didn't take into account that people would ever be using -- at the start of a class name or otherwise in selectors (even if it would be allowed) and focusing on the use as vars when -- is encountered.

Post by **Moonchild** » 2016-02-21, 01:07

Matt A Tobin wrote:A user came into irc and threw this code at me.. I don't understand the nature of this issue or anything.. The user also did not respond to further questions except to link this thread and that I should give this to Moonchild.

This one-liner change is wholly incomplete and would break more than it would fix. It's only part of the solution for a spec having changed out from under us.

Pale Moon forum

Including "--" class prefix in selector list prevents other selectors from matching

Including "--" class prefix in selector list prevents other selectors from matching

Re: Including "--" class prefix in selector list prevents other selectors from matching

Re: Including "--" class prefix in selector list prevents other selectors from matching

Re: Including "--" class prefix in selector list prevents other selectors from matching

Re: Including "--" class prefix in selector list prevents other selectors from matching

Re: Including "--" class prefix in selector list prevents other selectors from matching

Re: Including "--" class prefix in selector list prevents other selectors from matching