[webkit-reviews] review denied: [Bug 22441] Bridge the gap between the generated ElementFactory and HTMLElementFactory : [Attachment 25404] Second part: make the ElementFactory use PassRefPtr

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Nov 23 19:12:09 PST 2008


Darin Adler <darin at apple.com> has denied Julien Chaffraix
<jchaffraix at pleyo.com>'s request for review:
Bug 22441: Bridge the gap between the generated ElementFactory and
HTMLElementFactory
https://bugs.webkit.org/show_bug.cgi?id=22441

Attachment 25404: Second part: make the ElementFactory use PassRefPtr
https://bugs.webkit.org/attachment.cgi?id=25404&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +typedef PassRefPtr<$parameters{'namespace'}Element>
(*ConstructorFunc)(Document* doc, bool createdByParser);

There should not be a space between the ">" and the "(" symbols.

(These issues already existed.) The Document* argument should not have the name
"doc" -- the name should just be omitted. And the type should be named
ConstructorFunction, not ConstructorFunc. There's no value in abbreviating the
type name.

> +#include "config.h"

Headers should not include the "config.h" header; that's to be included by cpp
files only.

> +#ifndef $parameters{'namespace'}ElementFactory_h
> +#define $parameters{'namespace'}ElementFactory_h

The #ifndef part should go before the includes, not after them.

>      class $parameters{'namespace'}ElementFactory
>      {

(This issue already existed.) The brace should be on the same line as the class
name.

>      public:
> -	   WebCore::Element* createElement(const WebCore::QualifiedName& qName,
WebCore::Document* doc, bool createdByParser = true);
> -	   static $parameters{'namespace'}Element*
create$parameters{'namespace'}Element(const WebCore::QualifiedName& qName,
WebCore::Document* doc, bool createdByParser = true);
> +	   PassRefPtr<Element> createElement(const WebCore::QualifiedName&
qName, WebCore::Document* doc, bool createdByParser = true);
> +	   static PassRefPtr<$parameters{'namespace'}Element>
create$parameters{'namespace'}Element(const WebCore::QualifiedName& qName,
WebCore::Document* doc, bool createdByParser = true);

(This issue already existed.) The argument names qName and doc should be
omitted.

I don't see the change to the createElement definition.

review- because of the "cofig.h" issue


More information about the webkit-reviews mailing list