Changeset 73469 in webkit


Ignore:
Timestamp:
Dec 7, 2010 2:47:37 PM (13 years ago)
Author:
bweinstein@apple.com
Message:

Part of Layering Violation in ContextMenu
​https://bugs.webkit.org/show_bug.cgi?id=50586

Reviewed by John Sullivan.

WebCore:

ContextMenuMac.mm had a WebCoreMenuTarget class, which was responsible for wrapping
a ContextMenuItem, and calling through to the ContextMenuController to validate it
and call a function when it was selected.

It was a layering violation to have this class here, because ContextMenu can't know
about ContextMenuController.

This patch moves the class that wraps the ContextMenuItems to WebKit/mac/WebHTMLView.mm,
and sets up the menu targets there.

No change in behavior, no new tests.

  • WebCore.exp.in: Added function that needs to be exported.
  • platform/mac/ContextMenuMac.mm:

(WebCore::ContextMenu::ContextMenu): Don't set the sharedMenuTarget anymore.
(WebCore::ContextMenu::appendItem): Don't call setMenuItemTarget (this is done in WebKit now).
(WebCore::ContextMenu::insertItem): Ditto.

WebKit/mac:

Move WebMenuTarget from ContextMenuMac to here, because having it in ContextMenuMac
was a layering violation. Also, make sure we set the menu item targets for all menu
items before showing them, because the ContextMenu constructor doesn't do that anymore.

  • WebView/WebHTMLView.mm:

(+[WebMenuTarget sharedMenuTarget]): Moved from ContextMenuMac.mm.
(-[WebMenuTarget WebCore::]): Ditto.
(-[WebMenuTarget setMenuController:WebCore::]): Ditto.
(-[WebMenuTarget forwardContextMenuAction:]): Ditto.
(-[WebMenuTarget validateMenuItem:]): Ditto.

(setMenuItemTarget): Sets the target of the NSMenuItem to the shared WebMenuTarget.
(setMenuTargets): Recursively iterates over all NSMenuItems in an NSMenu (including

submenus), and calls setMenuItemTarget on them.

(-[WebHTMLView menuForEvent:]): Call setMenuTarget on all the menu items before adding

them to the menu.

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r73462 r73469  
     12010-12-07  Brian Weinstein  <bweinstein@apple.com>
     2
     3        Reviewed by John Sullivan.
     4
     5        Part of Layering Violation in ContextMenu
     6        https://bugs.webkit.org/show_bug.cgi?id=50586
     7       
     8        ContextMenuMac.mm had a WebCoreMenuTarget class, which was responsible for wrapping
     9        a ContextMenuItem, and calling through to the ContextMenuController to validate it
     10        and call a function when it was selected.
     11       
     12        It was a layering violation to have this class here, because ContextMenu can't know
     13        about ContextMenuController.
     14       
     15        This patch moves the class that wraps the ContextMenuItems to WebKit/mac/WebHTMLView.mm,
     16        and sets up the menu targets there.
     17
     18        No change in behavior, no new tests.
     19
     20        * WebCore.exp.in: Added function that needs to be exported.
     21        * platform/mac/ContextMenuMac.mm:
     22        (WebCore::ContextMenu::ContextMenu): Don't set the sharedMenuTarget anymore.
     23        (WebCore::ContextMenu::appendItem): Don't call setMenuItemTarget (this is done in WebKit now).
     24        (WebCore::ContextMenu::insertItem): Ditto.
     25
    1262010-12-07  Simon Fraser  <simon.fraser@apple.com>
    227

  • trunk/WebCore/WebCore.exp.in

    r73430 r73469  
    12501250__ZN7WebCore15ContextMenuItemC1ENS_19ContextMenuItemTypeENS_17ContextMenuActionERKN3WTF6StringEPNS_11ContextMenuE
    12511251__ZN7WebCore15ContextMenuItemC1ENS_19ContextMenuItemTypeENS_17ContextMenuActionERKN3WTF6StringEbb
     1252__ZN7WebCore15ContextMenuItemC1EP10NSMenuItem
    12521253__ZN7WebCore15ContextMenuItemD1Ev
    12531254__ZN7WebCore21ContextMenuController16clearContextMenuEv

  • trunk/WebCore/platform/mac/ContextMenuMac.mm

    r71682 r73469  
    2929#if ENABLE(CONTEXT_MENUS)
    3030
    31 #include "ContextMenuController.h"
    32 
    33 @interface WebCoreMenuTarget : NSObject {
    34     WebCore::ContextMenuController* _menuController;
    35 }
    36 + (WebCoreMenuTarget*)sharedMenuTarget;
    37 - (WebCore::ContextMenuController*)menuController;
    38 - (void)setMenuController:(WebCore::ContextMenuController*)menuController;
    39 - (void)forwardContextMenuAction:(id)sender;
    40 - (BOOL)validateMenuItem:(NSMenuItem *)item;
    41 @end
    42 
    43 static WebCoreMenuTarget* target;
    44 
    45 @implementation WebCoreMenuTarget
    46 
    47 + (WebCoreMenuTarget*)sharedMenuTarget
    48 {
    49     if (!target)
    50         target = [[WebCoreMenuTarget alloc] init];
    51     return target;
    52 }
    53 
    54 - (WebCore::ContextMenuController*)menuController
    55 {
    56     return _menuController;
    57 }
    58 
    59 - (void)setMenuController:(WebCore::ContextMenuController*)menuController
    60 {
    61     _menuController = menuController;
    62 }
    63 
    64 - (void)forwardContextMenuAction:(id)sender
    65 {
    66     WebCore::ContextMenuItem item(WebCore::ActionType, static_cast<WebCore::ContextMenuAction>([sender tag]), [sender title]);
    67     _menuController->contextMenuItemSelected(&item);
    68 }
    69 
    70 - (BOOL)validateMenuItem:(NSMenuItem *)item
    71 {
    72     WebCore::ContextMenuItem coreItem(item);
    73     ASSERT(_menuController->contextMenu());
    74     _menuController->contextMenu()->checkOrEnableIfNeeded(coreItem);
    75     return coreItem.enabled();
    76 }
    77 
    78 @end
    79 
    8031namespace WebCore {
    8132
    … …  
    8637    m_platformDescription = array;
    8738    [array release];
    88 
    89     [[WebCoreMenuTarget sharedMenuTarget] setMenuController:controller()];
    9039}
    9140
    … …  
    9443    , m_platformDescription(menu)
    9544{
    96     [[WebCoreMenuTarget sharedMenuTarget] setMenuController:controller()];
    9745}
    9846
    9947ContextMenu::~ContextMenu()
    10048{
    101 }
    102  
    103 static void setMenuItemTarget(NSMenuItem* menuItem)
    104 {
    105     [menuItem setTarget:[WebCoreMenuTarget sharedMenuTarget]];
    106     [menuItem setAction:@selector(forwardContextMenuAction:)];
    10749}
    10850
    … …  
    11153    checkOrEnableIfNeeded(item);
    11254
    113     ContextMenuItemType type = item.type();
    11455    NSMenuItem* platformItem = item.releasePlatformDescription();
    115     if (type == ActionType)
    116         setMenuItemTarget(platformItem);
    11756
    11857    [m_platformDescription.get() addObject:platformItem];
    … …  
    12463    checkOrEnableIfNeeded(item);
    12564
    126     ContextMenuItemType type = item.type();
    12765    NSMenuItem* platformItem = item.releasePlatformDescription();
    128     if (type == ActionType)
    129         setMenuItemTarget(platformItem);
    13066
    13167    [m_platformDescription.get() insertObject:platformItem atIndex:position];

  • trunk/WebKit/mac/ChangeLog

    r73444 r73469  
     12010-12-07  Brian Weinstein  <bweinstein@apple.com>
     2
     3        Reviewed by John Sullivan.
     4
     5        Part of Layering Violation in ContextMenu
     6        https://bugs.webkit.org/show_bug.cgi?id=50586
     7       
     8        Move WebMenuTarget from ContextMenuMac to here, because having it in ContextMenuMac
     9        was a layering violation. Also, make sure we set the menu item targets for all menu
     10        items before showing them, because the ContextMenu constructor doesn't do that anymore.
     11
     12        * WebView/WebHTMLView.mm:
     13        (+[WebMenuTarget sharedMenuTarget]): Moved from ContextMenuMac.mm.
     14        (-[WebMenuTarget WebCore::]): Ditto.
     15        (-[WebMenuTarget setMenuController:WebCore::]): Ditto.
     16        (-[WebMenuTarget forwardContextMenuAction:]): Ditto.
     17        (-[WebMenuTarget validateMenuItem:]): Ditto.
     18
     19        (setMenuItemTarget): Sets the target of the NSMenuItem to the shared WebMenuTarget.
     20        (setMenuTargets): Recursively iterates over all NSMenuItems in an NSMenu (including
     21            submenus), and calls setMenuItemTarget on them.
     22        (-[WebHTMLView menuForEvent:]): Call setMenuTarget on all the menu items before adding
     23            them to the menu.
     24
    1252010-12-06  Darin Adler  <darin@apple.com>
    226

  • trunk/WebKit/mac/WebView/WebHTMLView.mm

    r73337 r73469  
    129129using namespace WTF;
    130130using namespace std;
     131
     132@interface WebMenuTarget : NSObject {
     133    WebCore::ContextMenuController* _menuController;
     134}
     135+ (WebMenuTarget*)sharedMenuTarget;
     136- (WebCore::ContextMenuController*)menuController;
     137- (void)setMenuController:(WebCore::ContextMenuController*)menuController;
     138- (void)forwardContextMenuAction:(id)sender;
     139- (BOOL)validateMenuItem:(NSMenuItem *)item;
     140@end
     141
     142static WebMenuTarget* target;
     143
     144@implementation WebMenuTarget
     145
     146+ (WebMenuTarget*)sharedMenuTarget
     147{
     148    if (!target)
     149        target = [[WebMenuTarget alloc] init];
     150    return target;
     151}
     152
     153- (WebCore::ContextMenuController*)menuController
     154{
     155    return _menuController;
     156}
     157
     158- (void)setMenuController:(WebCore::ContextMenuController*)menuController
     159{
     160    _menuController = menuController;
     161}
     162
     163- (void)forwardContextMenuAction:(id)sender
     164{
     165    WebCore::ContextMenuItem item(WebCore::ActionType, static_cast<WebCore::ContextMenuAction>([sender tag]), [sender title]);
     166    _menuController->contextMenuItemSelected(&item);
     167}
     168
     169- (BOOL)validateMenuItem:(NSMenuItem *)item
     170{
     171    WebCore::ContextMenuItem coreItem(item);
     172    ASSERT(_menuController->contextMenu());
     173    _menuController->contextMenu()->checkOrEnableIfNeeded(coreItem);
     174    return coreItem.enabled();
     175}
     176
     177@end
    131178
    132179@interface NSWindow (BorderViewAccess)
    … …  
    32243271}
    32253272
     3273static void setMenuItemTarget(NSMenuItem* menuItem)
     3274{
     3275    [menuItem setTarget:[WebMenuTarget sharedMenuTarget]];
     3276    [menuItem setAction:@selector(forwardContextMenuAction:)];
     3277}
     3278
     3279static void setMenuTargets(NSMenu* menu)
     3280{
     3281    NSInteger itemCount = [menu numberOfItems];
     3282    for (NSInteger i = 0; i < itemCount; ++i) {
     3283        NSMenuItem *item = [menu itemAtIndex:i];
     3284        setMenuItemTarget(item);
     3285        if ([item hasSubmenu])
     3286            setMenuTargets([item submenu]);
     3287    }
     3288}
     3289
    32263290- (NSMenu *)menuForEvent:(NSEvent *)event
    32273291{
    … …  
    32713335    for (NSUInteger i = 0; i < count; i++)
    32723336        [menu addItem:[menuItems objectAtIndex:i]];
     3337    setMenuTargets(menu);
     3338   
     3339    [[WebMenuTarget sharedMenuTarget] setMenuController:page->contextMenuController()];
     3340   
    32733341    return menu;
    32743342}
Note: See TracChangeset for help on using the changeset viewer.