[webkit-reviews] review granted: [Bug 61276] <input type=color> Mac UI behaviour : [Attachment 207495] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Ruth Fong <ruthiecftg at gmail.com>'s
request for review:
Bug 61276: <input type=color> Mac UI behaviour
https://bugs.webkit.org/show_bug.cgi?id=61276

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=207495&action=review


> Source/WebCore/html/HTMLInputElement.cpp:1980
> +#if ENABLE(INPUT_TYPE_COLOR)
> +void HTMLInputElement::documentWillSuspendForPageCache()
> +{
> +    if (!isColorControl())
> +	   return;
> +    m_inputType.get()->detach();
> +}
> +#endif // ENABLE(INPUT_TYPE_COLOR)

This should go next to documentDidResumeFromPageCache.

> Source/WebCore/html/HTMLInputElement.h:307
> +#if ENABLE(INPUT_TYPE_COLOR)
> +    virtual void documentWillSuspendForPageCache() OVERRIDE;
> +#endif

This should go next to documentDidResumeFromPageCache().

> Source/WebKit2/UIProcess/mac/WebColorPickerMac.h:70
> +// A Listener class to act as a event target for NSColorPanel and send

Unnecessary capital L.

> Source/WebKit2/UIProcess/mac/WebColorPickerMac.h:74
> + at interface WKColorPanelMac : NSObject<NSWindowDelegate> {
> +    
> + at private

Unnecessary newline.

> Source/WebKit2/UIProcess/mac/WebColorPickerMac.mm:134
> +- (void)dealloc
> +{
> +    NSColorPanel *panel = [NSColorPanel sharedColorPanel];
> +    if ([panel delegate] == self) {
> +	   [panel setDelegate:nil];
> +	   [panel setTarget:nil];
> +	   [panel setAction:nil];
> +    }
> +
> +    [super dealloc];
> +}

You should move this to an invalidate method.