[webkit-reviews] review granted: [Bug 13707] REGRESSION: JavaScript exceptions on quotes.burntelectrons.org : [Attachment 14536] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 13707: REGRESSION: JavaScript exceptions on quotes.burntelectrons.org
http://bugs.webkit.org/show_bug.cgi?id=13707

Attachment 14536: proposed fix
http://bugs.webkit.org/attachment.cgi?id=14536&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks great.

+	 * dom/Document.idl: Additions mostly match HTML5 HTMLDocument
interface, except for
+	 of location, which is read-only in HTML5, and lastModified, which is
not present there.

"for of" here.

I'd like to see more of JSDocument::setLocation in a method named
Document::setLocation. We should do everything possible to have the language
bindings be only language issues. So I'd have the setLocation function take
parameters for the Frame* and the wasRunByUserGesture boolean, but have the
call to FrameLoader be in the document.

Not new code, but it's bad long term for us to use the idiom
frame->loader()->documentLoader() to get to the loader for this document,
because that could be the loader for the previous or next document in this
frame rather than the current one. I'd like that to be encapsulated in a
documentLoader() function so we can fix it in one place.

Why did the type of the return value for getElementsByName change? Internally
it can be useful for the types to be more specific than those in the IDL files.
Was there a problem compiling the JavaScript binding? If so, I guess that's the
best solution. Too bad.

-#if !defined(LANGUAGE_OBJECTIVE_C)
+#if defined(LANGUAGE_JAVASCRIPT)

I think we want these APIs in other languages. Why make them
JavaScript-specific?