[webkit-reviews] review denied: [Bug 105568] Implement AutocompleteErrorEvent#reason : [Attachment 181853] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Dan Beam <dbeam at chromium.org>'s
request for review:
Bug 105568: Implement AutocompleteErrorEvent#reason
https://bugs.webkit.org/show_bug.cgi?id=105568

Attachment 181853: Patch
https://bugs.webkit.org/attachment.cgi?id=181853&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=181853&action=review


> Source/WebCore/DerivedSources.cpp:36
> +#if ENABLE(REQUEST_AUTOCOMPLETE)
> +#include "JSAutocompleteErrorEvent.cpp"
> +#endif

You don't need to wrap with ENABLE(REQUEST_AUTOCOMPLETE) because the IDL has
Conditional=REQUEST_AUTOCOMPLETE.

> Source/WebCore/DerivedSources.make:202
> +    $(WebCore)/dom/AutocompleteErrorEvent.idl \

You need to add entries for the followings to
WebCore/WebCore.pbxproj/project.pbxproj:
 AutocompleteErrorEvent.cpp
 AutocompleteErrorEvent.h
 JSAutocompleteErrorEvent.cpp
 JSAutocompleteErrorEvent.h
 DOMAutocompleteErrorEvent.mm
 DOMAutocompleteErrorEvent.h


and add the followings to WebCore/WebCore.vcproj/WebCore.vcproj:
 JSAutocompleteErrorEvent.cpp
 JSAutocompleteErrorEvent.h

> Source/WebCore/dom/AutocompleteErrorEvent.h:14
> + *	  * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + *	  * Neither the name of Google Inc. nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS

This is not the standard copyright header for WebKit contribution.
This should follow Source/WebKit/LICENSE.

> Source/WebCore/dom/AutocompleteErrorEvent.h:30
> +#include "Event.h"

This file should be wrapped with #if ENABLE(REQUEST_AUTOCOMPLETE).

> Source/WebCore/dom/AutocompleteErrorEvent.h:38
> +    AutocompleteErrorEventInit()
> +    {
> +    };

Is this necessary?

> Source/WebCore/dom/DOMAllInOne.cpp:32
> +#if ENABLE(REQUEST_AUTOCOMPLETE)
> +#include "AutocompleteErrorEvent.cpp"
> +#endif

This is unnecessary.  We don't have AutocompleteErrorEvent.cpp.

> Source/WebCore/html/HTMLFormElement.cpp:65
> +#if ENABLE(REQUEST_AUTOCOMPLETE)
> +#include "AutocompleteErrorEvent.h"
> +#endif

You don't need to wrap with ENABLE(REQUEST_AUTOCOMPLETE) if the content of
AutocompleteErrorEvent.h is wrapped with it.