[webkit-reviews] review granted: [Bug 50586] Layering Violation in ContextMenu - member variable of type HitTestResult : [Attachment 75781] [PATCH] Fix + Qt (Take 2)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Dec 7 09:48:20 PST 2010
Darin Adler <darin at apple.com> has granted Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 50586: Layering Violation in ContextMenu - member variable of type
HitTestResult
https://bugs.webkit.org/show_bug.cgi?id=50586
Attachment 75781: [PATCH] Fix + Qt (Take 2)
https://bugs.webkit.org/attachment.cgi?id=75781&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=75781&action=review
> WebCore/page/ContextMenuController.cpp:79
> + , m_hitTestResult(HitTestResult(IntPoint()))
This seems a little strange. Do we really want to initialize a hit test result
with the point 0,0? Should we add a default constructor for HitTestResult? Or
maybe we should use OwnPtr<HitTestResult> and have it be 0 until we can
actually do the hit test.
> WebCore/page/ContextMenuController.cpp:139
> + return new ContextMenu();
No parentheses needed here when calling new with a constructor with no
arguments.
Can we change this function’s return type to PassOwnPtr?
> WebCore/page/ContextMenuController.h:79
> + void createAndAppendFontSubMenu(ContextMenuItem& fontMenuItem);
> + void createAndAppendSpellingAndGrammarSubMenu(ContextMenuItem&
spellingAndGrammarMenuItem);
> + void createAndAppendSpellingSubMenu(ContextMenuItem&
spellingMenuItem);
> + void createAndAppendSpeechSubMenu(ContextMenuItem& speechMenuItem);
> + void createAndAppendWritingDirectionSubMenu(ContextMenuItem&
writingDirectionMenuItem);
> + void createAndAppendTextDirectionSubMenu(ContextMenuItem&
textDirectionMenuItem);
> + void createAndAppendSubstitutionsSubMenu(ContextMenuItem&
substitutionsMenuItem);
> + void createAndAppendTransformationsSubMenu(ContextMenuItem&
transformationsMenuItem);
I’m not sure these arguments benefit from the names here.
More information about the webkit-reviews
mailing list