[Webkit-unassigned] [Bug 13830] Auto-generate JS DOM bindings for HTMLDocument and most of the rest of HTMLElement

[bugzilla-daemon at webkit.org]

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


darin at apple.com changed:

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




------- Comment #8 from darin at apple.com  2007-05-23 09:11 PDT -------
(From update of attachment 14675)
+        return toJS(exec, node);
+    } else if (!collection->length())
+        return jsUndefined();

No need for an else here.

+    if (Window* win = Window::retrieveWindow(frame))
+        return win->location();
+    return jsUndefined();

When can retrieveWindow return 0? Do we even need this case? Should it have
ASSERT_NOT_REACHED()?

+void JSHTMLDocument::setLocation(ExecState* exec, JSValue* value)
+{
+    if (Frame* frame = static_cast<HTMLDocument*>(impl())->frame()) {

I'd rather have an early return here than the entire function indented.

+        // When assigning location, IE and Mozilla both resolve the URL
+        // relative     not
+        // the target frame.

Formatting problem.

+        DeprecatedString str = value->toString(exec);

This can use String. Document has completeURL for both String and
DeprecatedString, so lets use the String version.

+void JSHTMLDocument::setLocation(ExecState* exec, JSValue* value)

Why does this need to be a custom binding. This seems to just call toString and
then act on the string. So it could use a standard binding if you added the
necessary function to HTMLDocument.

+JSValue* JSHTMLDocument::width(ExecState*) const
+JSValue* JSHTMLDocument::height(ExecState*) const
+JSValue* JSHTMLDocument::dir(ExecState*) const
+void JSHTMLDocument::setDir(ExecState* exec, JSValue* value)
+JSValue* JSHTMLDocument::designMode(ExecState*) const
+void JSHTMLDocument::setDesignMode(ExecState* exec, JSValue* value)
+JSValue* JSHTMLDocument::bgColor(ExecState*) const
+void JSHTMLDocument::setBgColor(ExecState* exec, JSValue* value)
+JSValue* JSHTMLDocument::fgColor(ExecState*) const
+void JSHTMLDocument::setFgColor(ExecState* exec, JSValue* value)
+JSValue* JSHTMLDocument::alinkColor(ExecState*) const
+void JSHTMLDocument::setAlinkColor(ExecState* exec, JSValue* value)
+JSValue* JSHTMLDocument::linkColor(ExecState*) const
+void JSHTMLDocument::setLinkColor(ExecState* exec, JSValue* value)
+JSValue* JSHTMLDocument::vlinkColor(ExecState*) const
+void JSHTMLDocument::setVlinkColor(ExecState* exec, JSValue* value)
+JSValue* JSHTMLDocument::clear(ExecState*, const List&)
+JSValue* JSHTMLDocument::captureEvents(ExecState*, const List&)
+JSValue* JSHTMLDocument::releaseEvents(ExecState*, const List&)

None of these seem to require a custom binding.

+            push(@specials, "DontDelete") unless
$attribute->signature->extendedAttributes->{"Deletatable"};

Should be "Deletable", not "Deletatable".

+#if defined(LANGUAGE_JAVASCRIPT)
+                 attribute DOMString domain;
+#else
         readonly attribute DOMString domain;
+#endif

Why do we need this to be read-only for non-JS languages?


-- 
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.