[Webkit-unassigned] [Bug 119356] [Forms: color] <input type='color'> popover color well implementation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119356


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #207957|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #22 from Benjamin Poulain <benjamin at webkit.org>  2013-08-01 17:29:51 PST ---
(From update of attachment 207957)
View in context: https://bugs.webkit.org/attachment.cgi?id=207957&action=review

> Source/WebKit2/Configurations/FeatureDefines.xcconfig:102
> +ENABLE_INPUT_TYPE_COLOR_POPOVER = ENABLE_INPUT_TYPE_COLOR_POPOVER;

You should probably update all 4 FeatureDefines.xcconfig to avoid future confusions.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:1009
> +    // Close popover color well when resizing window.

This comment does not add value.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:2938
> +    // A new popover color well needs to be created (and the previous one destroon each activation of a color element.

The comment should be in the #ifdef.

destroon? :)

Aaaaaarg, an unclosed parenthesis.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:2940
> +    m_colorPicker = nil;

nil -> 0

> Source/WebKit2/UIProcess/mac/WebColorPickerMac.h:74
> +    RetainPtr<NSObject<WKColorPickerUIMac> > m_colorPickerUI;

You can remove the space between the two '>' (Yay C++ 11).

> Source/WebKit2/UIProcess/mac/WebColorPickerMac.mm:106
> +    m_colorPickerUI = nullptr;

nullptr -> nil?

> Source/WebKit2/UIProcess/mac/WebColorPickerMac.mm:150
> +            return self;

Indentation.

> Source/WebKit2/UIProcess/mac/WebColorPickerMac.mm:200
> +        _lastChangedByUser = YES;
> +            return;
> +        }

Indentation.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.