[Webkit-unassigned] [Bug 13919] Autogenerate the JS bindings for the CSSRule and its subclasses

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=13919


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #14778|review?                     |review-
               Flag|                            |




------- Comment #4 from darin at apple.com  2007-05-30 07:32 PDT -------
(From update of attachment 14778)
         // FIXME: the DOM spec says that this attribute
         // should not be readonly.  The correct declaration
         // is commented out below.

This comment is no longer accurate, and so it should be removed.

+                 attribute [ConvertNullStringTo=Null, ConvertNullToNullString]
DOMString encoding
+                     /*setter raises(DOMException);*/

You can avoid commenting out the DOMException part by adding an ExceptionCode&
parameter to setEncoding.

Index: WebCore/css/CSSPageRule.h
===================================================================
--- WebCore/css/CSSPageRule.h   (revision 21870)
+++ WebCore/css/CSSPageRule.h   (working copy)
@@ -26,6 +26,7 @@

 #include "CSSRule.h"
 #include <wtf/RefPtr.h>
+#include "PlatformString.h"

It's not correct to include PlatformString.h in this header. You need
PlatformString.h to call selectorText(), setSelectorText(), or cssText() but
not to process the header. If there's a problem in the auto-generated code that
should be resolved in the auto-generation, not here.

                  // FIXME: DOM spec says that this can raise an
                  // exception on setting.
-                 attribute [ConvertNullToNullString] DOMString       
selectorText
+                 attribute [ConvertNullStringTo=Null, ConvertNullToNullString]
DOMString selectorText
                      /*setter raises(DOMException)*/;

                  // FIXME: DOM spec says that this can raise an
                  // exception on setting.
-                 attribute [ConvertNullToNullString] DOMString           
selectorText
+                 attribute [ConvertNullStringTo=Null, ConvertNullToNullString]
DOMString            selectorText
                      /*setter raises(DOMException)*/;

You can resolve these FIXMEs by adding an ExceptionCode& parameter to
setSelectorText.

-                 // FIXME: DOM spec says that this can raise an
+                 // FIXME: DOM spec says that this can be set and can raise an
                  // exception on setting.
+#if defined(LANGUAGE_JAVASCRIPT)
+        readonly attribute [ConvertNullToNullString] DOMString        cssText;
+#else
                  attribute [ConvertNullToNullString] DOMString        cssText
                      /*setter raises (DOMException)*/;
+#endif

Should add a setCSSText function with an ExceptionCode& parameter instead of
having an ifdef in the IDL.

Otherwise looks great.

I'm going to say review- because I want you to address at least some of these.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.