[Webkit-unassigned] [Bug 25639] Spellcheck disabling does not disable context menu

[bugzilla-daemon at webkit.org]

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





--- Comment #10 from Tony Chang (Google) <tony at chromium.org>  2010-05-13 19:36:28 PST ---
(In reply to comment #7)
> (From update of attachment 55937 [details])
> Comments in Skipped files start with "#" rather than "//".

Fixed.

> You have a TODO to add a bug number. Please do that.

Fixed.

> > +    if (subView) {
> > +        NSMenu* menu = [subView menuForEvent:event];
> > +        if (shouldPrintMenuItems) {
> > +            printf("ContextMenuItems: ");
> > +            for (int i = 0; i < [menu numberOfItems]; ++i) {
> 
> It doesn't make sense to set up the menu local variable before checking the shouldPrintMenuItems.

We always want to run [subView menuForEvent:event], so I put it outside the shouldPrintMenuItems check.

> > +                if (i > 0)
> > +                    printf(", ");
> > +                NSMenuItem* menuItem = [menu itemAtIndex:i];
> > +                printf("%s", [menuItem isSeparatorItem] ? "----" : [[menuItem title] UTF8String]);
> 
> Since menu item titles are localized, it might be better to log something else, perhaps the selector, reformatted in to the style of a WebCore editor command name.

I see.  I tried this using the code you provided, but I got a bunch of ForwardContextMenuAction and SubmenuAction.  It looks like it keeps the value in tag and there's a big switch statement for determining the action.  We could just write out the tag (int) or maybe have a switch to map enum values to strings (seems like it might be a pain to keep in sync).

The current patch has tag number + title, but it's mostly just to demonstrate the output.  Thoughts?

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