WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
13919
Autogenerate the JS bindings for the CSSRule and its subclasses
https://bugs.webkit.org/show_bug.cgi?id=13919
Summary
Autogenerate the JS bindings for the CSSRule and its subclasses
Sam Weinig
Reported
2007-05-29 20:21:07 PDT
Patch forthcoming.
Attachments
patch
(41.51 KB, patch)
2007-05-29 20:32 PDT
,
Sam Weinig
darin
: review-
Details
Formatted Diff
Diff
Updated Patch
(47.73 KB, patch)
2007-05-30 10:21 PDT
,
Sam Weinig
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Sam Weinig
Comment 1
2007-05-29 20:32:16 PDT
Created
attachment 14778
[details]
patch The only part of this patch that I am not sure of is in CSSRule.idl where I special cased JS for the cssText attribute. The hand rolled bindings had the attribute as readonly, but the spec and ObjC bindings don't have this restriction. On the otherhand, we don't really support it in the implementation so the issue is rather insignificant. A similar issue (but in the other direction) is CSSCharsetRule.idl.
Sam Weinig
Comment 2
2007-05-29 20:34:49 PDT
One note, this patch fixes a previously failing case in fast/dom/wrapper-classes.html
David Kilzer (:ddkilzer)
Comment 3
2007-05-29 21:21:29 PDT
***
Bug 13918
has been marked as a duplicate of this bug. ***
Darin Adler
Comment 4
2007-05-30 07:32:23 PDT
Comment on
attachment 14778
[details]
patch // 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.
Sam Weinig
Comment 5
2007-05-30 10:21:39 PDT
Created
attachment 14785
[details]
Updated Patch Patch addresses Darin's concerns and adds new files to WebCore.pro so as not to break the Qt and Gdk builds again.
Sam Weinig
Comment 6
2007-05-30 10:24:24 PDT
Urk, +void CSSPageRule::setSelectorText(const String& /*selectorText*/, ExceptionCode& /*ex*/) the /*ex*/ should be a /*ec*/
Darin Adler
Comment 7
2007-05-30 10:45:26 PDT
Comment on
attachment 14785
[details]
Updated Patch // Not part of the CSSOM What does that comment mean? r=me
Sam Weinig
Comment 8
2007-05-30 10:54:20 PDT
Landed in
r21896
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug