Bug 26609 - Support CSS3 attr() function
: Support CSS3 attr() function
Status: NEW
Product: WebKit
Classification: Unclassified
Component: CSS
: 528+ (Nightly build)
: All All
: P2 Enhancement
Assigned To: Nobody
:
Depends on:
Blocks: 25851
  Show dependency treegraph
 
Reported: 2009-06-22 10:19 PDT by Kai Brüning
Modified: 2014-09-04 10:29 PDT (History)
15 users (show)

See Also:


Attachments
A first attempt to implement the CSS3 attr() function (51.28 KB, patch)
2009-06-22 10:43 PDT, Kai Brüning
simon.fraser: review-
Details | Formatted Diff | Diff
test case for attr() function (997 bytes, text/html)
2011-05-07 01:31 PDT, Jos van den Oever
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kai Brüning 2009-06-22 10:19:34 PDT
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.
Comment 1 Kai Brüning 2009-06-22 10:43:25 PDT
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.
Comment 2 Eric Seidel 2009-08-08 09:14:29 PDT
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.
Comment 3 Eric Seidel 2009-08-08 09:32:28 PDT
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.
Comment 4 Kai Brüning 2009-08-17 10:59:29 PDT
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.
Comment 5 Eric Seidel 2009-08-17 18:22:06 PDT
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.
Comment 6 Simon Fraser (smfr) 2009-09-21 14:15:56 PDT
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.
Comment 7 Jos van den Oever 2011-05-07 01:31:22 PDT
Created attachment 92687 [details]
test case for attr() function

works in Firefox 4, not in WebKit