Bug 26609 – [css-values] Support advanced attr() functionality

• Component: CSS
• Status: NEW

[Kai Brüning]

CSS3 extends the attr() function substantially.

Especially the ability is added to set the content of an element to an image url contained in an arbitrary attribute. This is necessary to be able to display xml data with images which are not described by html <img> elements.

[Kai Brüning]

Created attachment 31653 [details]
A first attempt to implement the CSS3 attr() function

This patch is meant as a first attempt of an implementation. There are sure many things to change before landing it.

The up to three parameters of CSS3-attr() are fully supported, but attr() can not yet be used with all properties to which it would apply.

The patch introduces the concept of a "derived" value to CSSPrimitiveValue and CSSStyleSelector. The derived value is a new CSSPrimitiveValue, which is created only if the original value is of the newly added sub class CSSAttrValue. The derived value is then of the type required by the attr() function, for instance CSSImageValue.

Note that the patch includes namespace support for the attribute name (first parameter of attr()). For this I had to change CSSGrammar.y. Probably this won’t be the final change, because so far it does not support and empty prefix (that is, an attribute name of the form "|name").

Please advice how to proceed to get this landed eventually.

[Eric Seidel (no email)]

Comment on attachment 31653 [details]
A first attempt to implement the CSS3 attr() function

I think if I was trying to get this patch landed, I would start by writing some obvious tests and getting those landed.

You have tests, but they certainly could be made more obvious.  They can also be made dumpAsText using the js testing framework (found in fast/js/resources.  Look for TEMPLATE.html files and make-js-test-wrappers).

You can test matching css selectors from javascript using querySelectorAll or getComputedStyle().

Hum... but I just realized that I think that getComputedStyle().content is broken, so you might not actually be able to use getComputedStyle for this.  Bug 23668.  Hum.  I'll look at your patch again later in more detail.

[Eric Seidel (no email)]

Comment on attachment 31653 [details]
A first attempt to implement the CSS3 attr() function

Sorry for the unhelpful comments.  I was trying to suggest that it's difficult to tell if your tests pass or fail.  Mostly this is just a large, possibly complicated change, and no one has taken the time to decide if it's right or not yet.  Trying to make it more obvious to people that it's right (by making the tests more obvious) is one approach to getting it reviewed.  But I'll also just try and stare at it more early next week.

[Kai Brüning]

Eric, your suggestion certainly makes sense and I will work on the test cases soon.

Some additional information about the patch:

 I need this feature for a project which uses a custom build of WebKit (yes, we are aware of the caveats of doing so). To keep the merge effort minimal, I tried to do minimal changes for this patch.

To implement the complete functionality of CSS3 attr() it would probably make sense to refactor a little more in order to avoid a lot of boiler plate code duplication. I am willing to do this after getting feedback whether the patch is wanted at all and whether its approach is feasible.

[Eric Seidel (no email)]

You would have to ask Hyatt if "attr()" is a bad idea.  My general assumption is that we plan to most things from CSS3 where they do not negatively affect performance of the rest of the engine.

Mostly I think this patch is just large.  If you think it could be done in smaller changes with some up-front re-factoring, that's definitely encouraged. :)

If you can get Hyatt's attention in #webkit, he may be able to give you some quick feedback about the basics of this patch.

[Simon Fraser (smfr)]

Comment on attachment 31653 [details]
A first attempt to implement the CSS3 attr() function

r-ing the patch based on discussion in the bug, and the need to break it into smaller chunks, as well as add tests.

[Jos van den Oever]

Created attachment 92687 [details]
test case for attr() function

works in Firefox 4, not in WebKit

[Gérard Talbot (no longer involved)]

Tests 
-----

http://w3c-test.org/css/css-values/attr-color-invalid-cast.html

http://w3c-test.org/css/css-values/attr-color-valid.html

http://w3c-test.org/css/css-values/attr-invalid-type-008.html

http://w3c-test.org/css/css-values/attr-length-invalid-cast.html

http://w3c-test.org/css/css-values/attr-length-valid-zero-nofallback.html

http://w3c-test.org/css/css-values/attr-length-valid-zero.html

http://w3c-test.org/css/css-values/attr-length-valid.html

http://w3c-test.org/css/css-values/attr-px-invalid-cast.html

http://w3c-test.org/css/css-values/attr-px-valid.html

[Miriam Suzanne]

This could make a huge difference for CSS layout, completely replacing grid systems…

```css
[data-layout] {
  grid-area: attr(data-layout custom-ident, auto / 1 / auto / -1);
}
```

[Sam Sneddon [:gsnedders]]

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

[Sam Sneddon [:gsnedders]]

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

[Sam Sneddon [:gsnedders]]

<rdar://57256097>

[Karl Dubost]

There was a recent article about the feature. 
https://una.im/advanced-attr/

Some WPT failing in Safari currently. 
https://wpt.fyi/results/css/css-values?label=master&label=experimental&aligned&q=attr%20safari%3Afail