[Webkit-unassigned] [Bug 17366] ContextMenu::itemAtIndex() should be static and declared for Windows port only

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 20 11:30:20 PST 2008


http://bugs.webkit.org/show_bug.cgi?id=17366





------- Comment #3 from oleg.sukhodolsky at sun.com  2008-02-20 11:30 PDT -------
(In reply to comment #2)
> (From update of attachment 19127 [edit])
> I'd prefer to not have the #if in the header. There's no harm in declaring a
> function that isn't defined on all platforms, especially if we might want to
> define and use it on other platforms in the future. Is there something I'm
> overlooking here?

there are two changes in the patch:
- the method made static, since it here is no reason to have it non-static
- ifdef added to clarify the fact that this method is not used/needed by any
other platform (at least for now)

Both changes have the same purpose - clarify the API of ContextMenu.
Since you do not have documentation there it is very hard to understand what 
every method is supposed to do and whether it is needed at all.  So, these
suggested changes just add some infomration (you do not need this if you are 
not a Windows port and this method doesn need "this")

So this patch is about making code claerer, nothing else.

> +        static ContextMenuItem* itemAtIndex(unsigned, const
> PlatformMenuDescription);
> 
> The "const" here, while not new, is not helpful. It doesn't have any effect on
> the type of the parameter.

I have not added "const" I've just keeped it.  I can remove it if you want, btw 
there is at least one more places where "const PlatformMenuDescription" is used 
in this class.


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



More information about the webkit-unassigned mailing list