Bug 26609 - Support CSS3 attr() function
: Support CSS3 attr() function
Status: NEW
: WebKit
CSS
: 528+ (Nightly build)
: All All
: P2 Enhancement
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2009-06-22 10:19 PST by
Modified: 2012-11-24 20:06 PST (History)


Attachments
A first attempt to implement the CSS3 attr() function (51.28 KB, patch)
2009-06-22 10:43 PST, Kai Brüning
simon.fraser: review-
Review Patch | Details | Formatted Diff | Diff
test case for attr() function (997 bytes, text/html)
2011-05-07 01:31 PST, 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 From 2009-06-22 10:19:34 PST
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 From 2009-06-22 10:43:25 PST -------
Created an attachment (id=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 From 2009-08-08 09:14:29 PST -------
(From update of attachment 31653 [details])
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 From 2009-08-08 09:32:28 PST -------
(From update of attachment 31653 [details])
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 From 2009-08-17 10:59:29 PST -------
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 From 2009-08-17 18:22:06 PST -------
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 From 2009-09-21 14:15:56 PST -------
(From update of attachment 31653 [details])
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 From 2011-05-07 01:31:22 PST -------
Created an attachment (id=92687) [details]
test case for attr() function

works in Firefox 4, not in WebKit