[Webkit-unassigned] [Bug 81883] [WK2] Add style support for WebPopUpItem

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Alexis Menard (darktears) <alexis.menard at openbossa.org>  2012-03-27 13:15:22 PST ---
(From update of attachment 134011)
View in context: https://bugs.webkit.org/attachment.cgi?id=134011&action=review

r- to me.

> Source/WebKit2/GNUmakefile.am:441
> +        Source/WebKit2/Shared/WebPopupItemStyle.cpp \

Indentation issue.

> Source/WebKit2/Shared/WebPopupItem.cpp:-33
> -using namespace WebCore;

You need to keep that.

> Source/WebKit2/Shared/WebPopupItemStyle.h:45
> +    WebPopupItemStyle(bool visible, bool isDisplayNone, WebCore::TextDirection, bool hasTextDirectionOverride);

The names of the parameters are not adding anything in the .h file, remove them

> Source/WebKit2/WebProcess/WebCoreSupport/WebPopupMenu.cpp:92
> +            WebPopupItemStyle style = WebPopupItemStyle(itemStyle.isVisible(), itemStyle.isDisplayNone(), itemStyle.textDirection(), itemStyle.hasTextDirectionOverride());

Lot of copies no? The affection and then in WebPopupItem constructor it will copy. Could you save one at least? WebPopupItemStyle style(....).

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