Bug 5153 – deleteRule and insertRule do not work

• Component: CSS
• Status: RESOLVED FIXED

[Mihai Parparita]

The linked URL is a simple test case for inserting and deleting rules from a
stylesheet object.  

Expected behavior (as observed in Firefox):
- clicking on the "x.deleteRule(0)" link causes the red text to become purple
- clicking on the "x.insertRule..." link causes the text to have a black border

Observed behavior (as observed in Safari 312.3):
- No visible changes occur (and no errors are reported in the JavaScript console)

[Alexey Proskuryakov]

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

[Anders Carlsson]

Created attachment 5620 [details]
Fix

[Darin Adler]

Comment on attachment 5620 [details]
Fix

+  removeRule	 DOMCSSStyleSheet::RemoveRule	 DontDelete|Function 0

Number of parameters here should be 1, not 0.

This patch makes two fixes. One is about updating the disabled flag on style
sheets. That fix has no layout test. I think that fix should be landed
separately, but it might be OK to land them together if there was a test for
it.

Otherwise, looks great.

[Anders Carlsson]

(In reply to comment #3)
> (From update of attachment 5620 [details] [edit])
> +  removeRule	 DOMCSSStyleSheet::RemoveRule	 DontDelete|Function 0
> 
> Number of parameters here should be 1, not 0.
> 

Isn't the number the minimum required number? The MSDN documentation on removeRule says:

iIndex	 Optional. Integer that specifies the index value of the rule to be deleted from the style sheet. 
If an index is not provided, the first rule in the rules collection is removed.


> This patch makes two fixes. One is about updating the disabled flag on style
> sheets. That fix has no layout test. I think that fix should be landed
> separately, but it might be OK to land them together if there was a test for
> it.

It actually has a test:

var s3 = document.getElementById('style3').sheet;
s3.disabled = true;

> 
> Otherwise, looks great.
> 

[Anders Carlsson]

Comment on attachment 5620 [details]
Fix

I'll mark it as ? again

[Darin Adler]

Comment on attachment 5620 [details]
Fix

I still think the number of parameters for removeRule is wrong; we have to
check with WinIE. This patch is otherwise great.

I think I'll mark this review+ and let Anders investigate that one last part.

[Anders Carlsson]

(In reply to comment #6)
> (From update of attachment 5620 [details] [edit])
> I still think the number of parameters for removeRule is wrong; we have to
> check with WinIE. This patch is otherwise great.
> 
> I think I'll mark this review+ and let Anders investigate that one last part.
> 

Apparently WinIE doesn't have a length property on the removeRule function. I changed it to 1 and 
landed the patch.