Bug 3534 – CSSMutableStyleDeclarationImpl::item is unimplemented

• Component: DOM
• Status: RESOLVED FIXED

[Dan Wood]

CSSMutableStyleDeclarationImpl::item (in css_valueimpl.cpp) is not defined.  This is called by 
[DOMCSSStyleDeclaration item:] in WebKit (DOM-CSS.mm).

[Joost de Valk (AlthA)]

Could you please point out to why this is a problem and what it breaks? Maybe even attach a testcase? :)

[Dan Wood]

The webkit method -[DOMCSSStyleDeclaration item:] calls CSSStyleDeclarationImpl::item...

- (DOMStyleSheet *)item:(unsigned long)index
{
    return [DOMStyleSheet _DOMStyleSheetWithImpl:[self _styleSheetListImpl]->item(index)];
}

This is the implementation of CSSStyleDeclarationImpl::item; notice that the index is unused and that 
an empty string is returned.

DOMString CSSStyleDeclarationImpl::item( unsigned long /*index*/ )
{
    // ###
    //return m_lstValues->at(index);
    return DOMString();
}


So for instance, if you were trying to process a DOMElement's style attribute (say, to make some 
modifications or output the style), by looping though each item (simplified snippet below), you'd find 
that the "item" method returns an empty string, so you couldn't process the style item.

				DOMCSSStyleDeclaration *style = [self style];
				NSLog(@"style = %@", style);
				int length = [style length];
				id item = [style item:0];

[Joost de Valk (AlthA)]

we're getting there, if you'd please make a testcase out of your small example which behaves incorrect 
and for example works in firefox, then we have a case here :)

[Joost de Valk (AlthA)]

hyatt	   AlthA: he's pointed out the function does not contain an impl
hyatt	   AlthA: thats enough to confirm it's a bug
hyatt   AlthA: even without a testcase

confirmed.

[Anders Carlsson]

Created attachment 2430 [details]
Test case

Here's a test case that shows that item returns an empty string

[Anders Carlsson]

Created attachment 2437 [details]
Return the correct property string

Here's a patch that makes ::item return the correct property string. 

I do know that m_values[i] is O(n), but I don't really think that it matters
much here because the number of style rules is usually rather small and I'd
guess that ::item is seldomly used.

[Darin Adler]

Comment on attachment 2437 [details]
Return the correct property string

Looks fine. If it was me I would have formatted the
CSSMutableStyleDeclarationImpl::item function a little tighter, more like
CSSComputedStyleDeclarationImpl::item, but that's such a nit. Lets land this
sucker.

[Joost de Valk (AlthA)]

Reporter, i'll let you do the honors of verifying this one, since you know how well embedded i am in the 
code *ugh*.

[Joost de Valk (AlthA)]

Dan, please verify this one if it has been fixed for you.

[Dan Wood]

(In reply to comment #9)
> Dan, please verify this one if it has been fixed for you.

Well, something is definitely implemented, but I don't know if what is implemented is correct.  

I wrote a little test code that gets a computed style, and then outputs the items in the 
DOMCSSStyleDeclaration....


	DOMCSSStyleDeclaration *style = [[self DOMDocument] getComputedStyle:(DOMElement *)
node :@""];

	NSMutableString *string = [NSMutableString string];
	int i;
	for (i = 0 ; i < [style length] ; i++ )
	{
		[string appendString:[style item:i]];
		[string appendString:@" "];
	}
	NSLog(@"items (%d) = %@", [style length], string);


What results is this:

2005-07-07 10:59:21.369 Sandvox[21119] items (99) = rgba(0, 0, 0, 0) none repeat scroll 0 0 auto 
auto separate 0px 0px 0px 0px rgba(0, 0, 0, 0) rgba(0, 0, 0, 0) rgba(0, 0, 0, 0) rgba(0, 0, 0, 0) none 
none none none 0px 0px 0px 0px auto stretch normal 0 1 single 1 horizontal start top none rgb(0, 0, 
0) auto dashboard-region( ltr inline show none Futura 40.128px normal normal normal 0px auto 
normal after-white-space -1% normal none outside disc 0px 0px 0px 0px auto 6px infinite scroll -1px 
-1px 0px 0px space 1 2 none visible auto auto auto auto auto auto auto static auto auto center none 
none 0px rgb(255, 255, 255) 0px 1px 1px uppercase auto normal read-write baseline visible normal 2 
0px 0px break-word normal 


Wow, this doesn't look right.  Or maybe it is.  It's hard to tell because the "cssText" method is not 
implemented either (new bug #3893), so I don't have anything to compare it to.  But it seems like it's 
only the "value" of a property.  I can't find any documentation on what this is *supposed* to return, but 
it doesn't look like it's anything useful.

So I'd like it if somebody who is more famiiar with what this is supposed to do can verify the bug, 
please.

[Anders Carlsson]

The problem is that CSSComputedStyleDeclarationImpl::item() isn't implemented. I've uploaded a patch for 
bug 3983 that implements it, so I'll just close this bug.