Bug 7492 – CSS attribute selectors fail when the setAttribute() function is used to create an attribute and the attribute's name is not "class" or "id"

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Chris Bentley]

Summary:
When an element's attribute is created using the setAttribute() method then only the attributes named "class" and "id" affect the rendering by associated CSS Attribute Selector rules. The CSS ignores any other attribute name.

Steps to reproduce:
The page at http://code.clientside.net.au/safari/setAttribute.html demonstrates the issue.

Expected behaviour:
I expect Safari to style an element using a CSS Attribute Selector when the attribute has been created using setAttribute() . eg. setAttribute('title','This is a tooltip') // el[title]

Actual Behaviour:
Only the "class" and "id" attributes affect the CSS rendering of an element when the attribute is created using the setAttribute() method in Safari.

Workaround:
None found.

Isolation:
Fails on Safari (except with class and id attributes). Firefox 1.5 and Opera 8.5 (Mac) behave as expected.

[David Kilzer (:ddkilzer)]

Created attachment 6759 [details]
Test case

Test case from URL provided.

[David Kilzer (:ddkilzer)]

Adding HasReduction keyword.  This is broken in Safari 2.0.3 (417.8) in 10.4.5 as well as the WebKit nightlies (locally built r12990), and thus is not a regression.

[Rob Buis]

Created attachment 8165 [details]
Proposed patch

The patch simply checks on attribute change whether there are any attribute selectors, if
so it marks the element as changed. It seems to me this is not needed for mapped attributes,
hence the entry == eNone check.
Cheers,

Rob.

[Darin Adler]

Comment on attachment 8165 [details]
Proposed patch

Looks to me like this is one for Hyatt to review.

[Maciej Stachowiak]

I don't think this fix is complete. An attribute selector could change the element from display:none to not, and when dispay:none, the element won't have a renderer. I suspect a better approach is to keep a hashtable of attribute names where there is a related attribute selector, and mark the element as changed whenever an attribute in that set is changed.

[Maciej Stachowiak]

Created attachment 8178 [details]
test case that I suspect won't work w/ this patch

[Maciej Stachowiak]

Comment on attachment 8165 [details]
Proposed patch

r- because I don't think it will work in the case posted. I didn't try it though.

[Rob Buis]

(In reply to comment #6)
> Created an attachment (id=8178) [edit]
> test case that I suspect won't work w/ this patch
> 

I checked it and it does not work using my current patch. Though
I assume it does not work without my patch either ;) Anyway, the
hashtable approach sounds good to me, I'll work on it soon.
Cheers,

Rob.

[Rob Buis]

Created attachment 8202 [details]
New approach

This new patch uses a Hashset. If the patch gets okayed, it is probably
easy for me to merge the original testcase with Maciej's to make a good
testcase for the patch.
Cheers,

Rob.

[Rob Buis]

Created attachment 8225 [details]
Now includes a testcase!

I made some name changes after review from hyatt and othermaciej. Also
I made a new testcase out of the original one, combined with Maciej's example
testcase.
Cheers,

Rob.

[Maciej Stachowiak]

Comment on attachment 8225 [details]
Now includes a testcase!

r=me

[Darin Adler]

Had to move the test, because it does not belong in the css2.1 directory, and had to generate test results, not included here.

Committed revision 14433.