[webkit-reviews] review granted: [Bug 40484] Add beforeScript (or equivalent) event to fire on inline scripts and inline stylesheets : [Attachment 59805] IDL and boilerplate, build system stuff, etc - actual implementation to follow in a second patch.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 40484: Add beforeScript (or equivalent) event to fire on inline scripts and
inline stylesheets
https://bugs.webkit.org/show_bug.cgi?id=40484

Attachment 59805: IDL and boilerplate, build system stuff, etc - actual
implementation to follow in a second patch.
https://bugs.webkit.org/attachment.cgi?id=59805&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
>      BeforeLoadEvent \
> +	BeforeProcessEvent \
>      Blob \

There's a tab here, but the other lines use spaces.

> +const String& BeforeProcessEvent::text() const
> +{
> +    // FIXME - Return innerText for <style> elements and inline <script>
elements, or the resource text for remote <script> elements
> +    static String* placeholder = new String;
> +    return *placeholder;
> +}

Can this really return const String& in all cases? Never needs to be a new
string?

> +	   return adoptRef(new BeforeProcessEvent());

No need for the parentheses here after the class name.

> +    void initBeforeProcessEvent(const AtomicString& type, bool canBubble,
bool cancelable)
> +    {
> +	   if (dispatched())
> +	       return;
> +	   
> +	   initEvent(type, canBubble, cancelable);
> +    }

Why the dispatched() check? Doesn't initEvent already check that?

> +private:
> +    BeforeProcessEvent()
> +	   : Event(eventNames().beforeprocessEvent, false, true)
> +    {}

Braces should be on separate lines.

> +	   void initBeforeProcessEvent(in DOMString type,
> +				    in boolean canBubble, 
> +				    in boolean cancelable);

You should just put this all on one line. This indenting style isn't good, even
if you copied it from another IDL file.

Otherwise looks OK. Seems fine to land.