Changeset 74580 in webkit


Ignore:
Timestamp:
Dec 23, 2010 2:20:43 PM (13 years ago)
Author:
Darin Adler
Message:

2010-12-23 Darin Adler <Darin Adler>

Reviewed by Sam Weinig.

WKView should not try to do asynchronous validation for selectors that are not editor commands
​https://bugs.webkit.org/show_bug.cgi?id=51555

  • WebCore.exp.in: Added commandIsSupportedFromMenuOrKeyBinding.
  • editing/Editor.h: Reordered arguments in the Editor::Command constructor and the data members too so the frame is last. Added commandIsSupportedFromMenuOrKeyBinding.
  • editing/EditorCommand.cpp: (WebCore::supported): Removed the EditorCommandSource argument. These functions are now only used when called from DOM. (WebCore::supportedFromMenuOrKeyBinding): Ditto. (WebCore::supportedCopyCut): Ditto. (WebCore::supportedPaste): Ditto. (WebCore::enabledDismissCorrectionPanel): Changed the supported function to an enabled function. It was incorrect to say that this is "supported" only when the correction panel is up. Correct to say that it is "enabled" only then. And also probably OK to enable it even when the selection is not in editable text, as long as the panel is up. (WebCore::createCommandMap): Moved conditional commands out of the main array into a separate section at the end. (WebCore::internalCommand): Added. (WebCore::Editor::command): Changed to use the new internalCommand function and simplified by relying on the null check in the Command constructor. (WebCore::Editor::commandIsSupportedFromMenuOrKeyBinding): Added. (WebCore::Editor::Command::Command): Removed unneeded initialization of m_source, which is never looked at if m_command is 0. Added feature of passing a null command pointer to the non-default constructor. (WebCore::Editor::Command::isSupported): Changed to only call the per-command isSupported function when the command source is DOM. Accordingly that function is now called isSupportedFromDOM.

2010-12-23 Darin Adler <Darin Adler>

Reviewed by Sam Weinig.

WKView should not try to do asynchronous validation for selectors that are not editor commands
​https://bugs.webkit.org/show_bug.cgi?id=51555

  • UIProcess/API/mac/WKView.mm: (-[WKView validateUserInterfaceItem:]): Removed the special case for startSpeaking. Added call to commandIsSupportedFromMenuOrKeyBinding so we only try to do validation for commands that are supported. Tweaked comments and added some bug numbers. (-[WKView _setUserInterfaceItemState:enabled:state:]): Tweaked comment and added bug number.
Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r74579 r74580  
     12010-12-23  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Sam Weinig.
     4
     5        WKView should not try to do asynchronous validation for selectors that are not editor commands
     6        https://bugs.webkit.org/show_bug.cgi?id=51555
     7
     8        * WebCore.exp.in: Added commandIsSupportedFromMenuOrKeyBinding.
     9        * editing/Editor.h: Reordered arguments in the Editor::Command constructor
     10        and the data members too so the frame is last. Added
     11        commandIsSupportedFromMenuOrKeyBinding.
     12
     13        * editing/EditorCommand.cpp:
     14        (WebCore::supported): Removed the EditorCommandSource argument. These
     15        functions are now only used when called from DOM.
     16        (WebCore::supportedFromMenuOrKeyBinding): Ditto.
     17        (WebCore::supportedCopyCut): Ditto.
     18        (WebCore::supportedPaste): Ditto.
     19        (WebCore::enabledDismissCorrectionPanel): Changed the supported function to
     20        an enabled function. It was incorrect to say that this is "supported" only
     21        when the correction panel is up. Correct to say that it is "enabled" only
     22        then. And also probably OK to enable it even when the selection is not in
     23        editable text, as long as the panel is up.
     24        (WebCore::createCommandMap): Moved conditional commands out of the main
     25        array into a separate section at the end.
     26        (WebCore::internalCommand): Added.
     27        (WebCore::Editor::command): Changed to use the new internalCommand function
     28        and simplified by relying on the null check in the Command constructor.
     29        (WebCore::Editor::commandIsSupportedFromMenuOrKeyBinding): Added.
     30        (WebCore::Editor::Command::Command): Removed unneeded initialization of
     31        m_source, which is never looked at if m_command is 0. Added feature of
     32        passing a null command pointer to the non-default constructor.
     33        (WebCore::Editor::Command::isSupported): Changed to only call the
     34        per-command isSupported function when the command source is DOM.
     35        Accordingly that function is now called isSupportedFromDOM.
     36
    1372010-12-23  Matthew Delaney  <mdelaney@apple.com>
    238

  • trunk/WebCore/WebCore.exp.in

    r74566 r74580  
    716716__ZN7WebCore6Editor35increaseSelectionListLevelUnorderedEv
    717717__ZN7WebCore6Editor35setIgnoreCompositionSelectionChangeEb
     718__ZN7WebCore6Editor38commandIsSupportedFromMenuOrKeyBindingERKN3WTF6StringE
    718719__ZN7WebCore6Editor3cutEv
    719720__ZN7WebCore6Editor44confirmCompositionWithoutDisturbingSelectionEv

  • trunk/WebCore/editing/Editor.h

    r74566 r74580  
    176176    public:
    177177        Command();
    178         Command(PassRefPtr<Frame>, const EditorInternalCommand*, EditorCommandSource);
     178        Command(const EditorInternalCommand*, EditorCommandSource, PassRefPtr<Frame>);
    179179
    180180        bool execute(const String& parameter = String(), Event* triggeringEvent = 0) const;
    … …  
    190190
    191191    private:
    192         RefPtr<Frame> m_frame;
    193192        const EditorInternalCommand* m_command;
    194193        EditorCommandSource m_source;
     194        RefPtr<Frame> m_frame;
    195195    };
    196     Command command(const String& commandName); // Default is CommandFromMenuOrKeyBinding.
     196    Command command(const String& commandName); // Command source is CommandFromMenuOrKeyBinding.
    197197    Command command(const String& commandName, EditorCommandSource);
     198    static bool commandIsSupportedFromMenuOrKeyBinding(const String& commandName); // Works without a frame.
    198199
    199200    bool insertText(const String&, Event* triggeringEvent);

  • trunk/WebCore/editing/EditorCommand.cpp

    r74566 r74580  
    6767public:
    6868    bool (*execute)(Frame*, Event*, EditorCommandSource, const String&);
    69     bool (*isSupported)(Frame*, EditorCommandSource);
     69    bool (*isSupportedFromDOM)(Frame*);
    7070    bool (*isEnabled)(Frame*, Event*, EditorCommandSource);
    7171    TriState (*state)(Frame*, Event*);
    … …  
    10891089// Supported functions
    10901090
    1091 static bool supported(Frame*, EditorCommandSource)
    1092 {
    1093     return true;
    1094 }
    1095 
    1096 static bool supportedFromMenuOrKeyBinding(Frame*, EditorCommandSource source)
    1097 {
    1098     return source == CommandFromMenuOrKeyBinding;
    1099 }
    1100 
    1101 static bool supportedCopyCut(Frame* frame, EditorCommandSource source)
    1102 {
    1103     switch (source) {
    1104     case CommandFromMenuOrKeyBinding:
    1105         return true;
    1106     case CommandFromDOM:
    1107     case CommandFromDOMWithUserInterface: {
    1108         Settings* settings = frame ? frame->settings() : 0;
    1109         return settings && settings->javaScriptCanAccessClipboard();
    1110     }
    1111     }
    1112     ASSERT_NOT_REACHED();
     1091static bool supported(Frame*)
     1092{
     1093    return true;
     1094}
     1095
     1096static bool supportedFromMenuOrKeyBinding(Frame*)
     1097{
    11131098    return false;
    11141099}
    11151100
    1116 static bool supportedPaste(Frame* frame, EditorCommandSource source)
    1117 {
    1118     switch (source) {
    1119     case CommandFromMenuOrKeyBinding:
    1120         return true;
    1121     case CommandFromDOM:
    1122     case CommandFromDOMWithUserInterface: {
    1123         Settings* settings = frame ? frame->settings() : 0;
    1124         return settings && (settings->javaScriptCanAccessClipboard() ? settings->isDOMPasteAllowed() : 0);
    1125     }
    1126     }
    1127     ASSERT_NOT_REACHED();
    1128     return false;
    1129 }
    1130 
    1131 #if SUPPORT_AUTOCORRECTION_PANEL
    1132 static bool supportedDismissCorrectionPanel(Frame* frame, EditorCommandSource source)
    1133 {
    1134     return supportedFromMenuOrKeyBinding(frame, source) && frame->editor()->isShowingCorrectionPanel();
    1135 }
    1136 #endif
     1101static bool supportedCopyCut(Frame* frame)
     1102{
     1103    Settings* settings = frame ? frame->settings() : 0;
     1104    return settings && settings->javaScriptCanAccessClipboard();
     1105}
     1106
     1107static bool supportedPaste(Frame* frame)
     1108{
     1109    Settings* settings = frame ? frame->settings() : 0;
     1110    return settings && (settings->javaScriptCanAccessClipboard() ? settings->isDOMPasteAllowed() : 0);
     1111}
    11371112
    11381113// Enabled functions
    … …  
    12491224    return frame->editor()->canUndo();
    12501225}
     1226
     1227#if SUPPORT_AUTOCORRECTION_PANEL
     1228static bool enabledDismissCorrectionPanel(Frame* frame, EditorCommandSource source)
     1229{
     1230    return frame->editor()->isShowingCorrectionPanel();
     1231}
     1232#endif
    12511233
    12521234// State functions
    … …  
    15071489        { "Superscript", { executeSuperscript, supported, enabledInRichlyEditableText, stateSuperscript, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
    15081490        { "SwapWithMark", { executeSwapWithMark, supportedFromMenuOrKeyBinding, enabledVisibleSelectionAndMark, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
    1509 #if PLATFORM(MAC)
    1510         { "TakeFindStringFromSelection", { executeTakeFindStringFromSelection, supportedFromMenuOrKeyBinding, enabledTakeFindStringFromSelection, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
    1511 #endif
    15121491        { "ToggleBold", { executeToggleBold, supportedFromMenuOrKeyBinding, enabledInRichlyEditableText, stateBold, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
    15131492        { "ToggleItalic", { executeToggleItalic, supportedFromMenuOrKeyBinding, enabledInRichlyEditableText, stateItalic, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
    … …  
    15211500        { "Yank", { executeYank, supportedFromMenuOrKeyBinding, enabledInEditableText, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
    15221501        { "YankAndSelect", { executeYankAndSelect, supportedFromMenuOrKeyBinding, enabledInEditableText, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
     1502
     1503#if PLATFORM(MAC)
     1504        { "TakeFindStringFromSelection", { executeTakeFindStringFromSelection, supportedFromMenuOrKeyBinding, enabledTakeFindStringFromSelection, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
     1505#endif
     1506
    15231507#if SUPPORT_AUTOCORRECTION_PANEL
    1524         { "CancelOperation", { executeCancelOperation, supportedDismissCorrectionPanel, enabledInEditableText, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
     1508        { "CancelOperation", { executeCancelOperation, supportedFromMenuOrKeyBinding, enabledDismissCorrectionPanel, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
    15251509#endif
    15261510    };
    … …  
    15831567}
    15841568
     1569static const EditorInternalCommand* internalCommand(const String& commandName)
     1570{
     1571    static const CommandMap& commandMap = createCommandMap();
     1572    return commandMap.get(commandName);
     1573}
     1574
    15851575Editor::Command Editor::command(const String& commandName)
    15861576{
    1587     return command(commandName, CommandFromMenuOrKeyBinding);
     1577    return Command(internalCommand(commandName), CommandFromMenuOrKeyBinding, m_frame);
    15881578}
    15891579
    15901580Editor::Command Editor::command(const String& commandName, EditorCommandSource source)
    15911581{
    1592     if (commandName.isEmpty())
    1593         return Command();
    1594 
    1595     static const CommandMap& commandMap = createCommandMap();
    1596     const EditorInternalCommand* internalCommand = commandMap.get(commandName);
    1597     return internalCommand ? Command(m_frame, internalCommand, source) : Command();
     1582    return Command(internalCommand(commandName), source, m_frame);
     1583}
     1584
     1585bool Editor::commandIsSupportedFromMenuOrKeyBinding(const String& commandName)
     1586{
     1587    return internalCommand(commandName);
    15981588}
    15991589
    16001590Editor::Command::Command()
    16011591    : m_command(0)
    1602     , m_source()
    1603 {
    1604 }
    1605 
    1606 Editor::Command::Command(PassRefPtr<Frame> frame, const EditorInternalCommand* command, EditorCommandSource source)
    1607     : m_frame(frame)
    1608     , m_command(command)
     1592{
     1593}
     1594
     1595Editor::Command::Command(const EditorInternalCommand* command, EditorCommandSource source, PassRefPtr<Frame> frame)
     1596    : m_command(command)
    16091597    , m_source(source)
    1610 {
    1611     ASSERT(m_frame);
    1612     ASSERT(m_command);
     1598    , m_frame(command ? frame : 0)
     1599{
     1600    // Use separate assertions so we can tell which bad thing happened.
     1601    if (!command)
     1602        ASSERT(!m_frame);
     1603    else
     1604        ASSERT(m_frame);
    16131605}
    16141606
    … …  
    16311623bool Editor::Command::isSupported() const
    16321624{
    1633     return m_command && m_command->isSupported(m_frame.get(), m_source);
     1625    if (!m_command)
     1626        return false;
     1627    switch (m_source) {
     1628    case CommandFromMenuOrKeyBinding:
     1629        return true;
     1630    case CommandFromDOM:
     1631    case CommandFromDOMWithUserInterface:
     1632        return m_command->isSupportedFromDOM(m_frame.get());
     1633    }
     1634    ASSERT_NOT_REACHED();
     1635    return false;
    16341636}
    16351637

  • trunk/WebKit2/ChangeLog

    r74575 r74580  
     12010-12-23  Darin Adler  <darin@apple.com>
     2
     3        Reviewed by Sam Weinig.
     4
     5        WKView should not try to do asynchronous validation for selectors that are not editor commands
     6        https://bugs.webkit.org/show_bug.cgi?id=51555
     7
     8        * UIProcess/API/mac/WKView.mm:
     9        (-[WKView validateUserInterfaceItem:]): Removed the special case for startSpeaking.
     10        Added call to commandIsSupportedFromMenuOrKeyBinding so we only try to do validation
     11        for commands that are supported. Tweaked comments and added some bug numbers.
     12        (-[WKView _setUserInterfaceItemState:enabled:state:]): Tweaked comment and added
     13        bug number.
     14
    1152010-12-23  Sam Weinig  <sam@webkit.org>
    216

  • trunk/WebKit2/UIProcess/API/mac/WKView.mm

    r74521 r74580  
    336336    SEL action = [item action];
    337337
    338     // FIXME: This is only needed to work around the fact that we don't return YES for
    339     // selectors that are not editing commands (see below). If that's fixed, this can be removed.
    340     if (action == @selector(startSpeaking:))
    341         return YES;
    342 
    343338    if (action == @selector(stopSpeaking:))
    344339        return [NSApp isSpeaking];
    … …  
    357352    }
    358353
    359     // FIXME: We should return YES here for selectors that are not editing commands.
    360     // But at the moment there is no way to find out if a selector is an editing command or not
    361     // in the UI process. So for now we assume any selectors not handled above are editing commands.
    362 
     354    // Next, handle editor commands. Start by returning YES for anything that is not an editor command.
     355    // Returning YES is the default thing to do in an AppKit validate method for any selector that is not recognized.
    363356    String commandName = commandNameForSelector([item action]);
    364     if (commandName.isEmpty())
     357    if (!Editor::commandIsSupportedFromMenuOrKeyBinding(commandName))
    365358        return YES;
    366359
    367     // Add this menu item to the vector of items for a given command that are awaiting validation.
    368     // If the item is the first to be added, then call validateMenuItem to start the asynchronous
    369     // validation process.
     360    // Add this item to the vector of items for a given command that are awaiting validation.
    370361    pair<ValidationMap::iterator, bool> addResult = _data->_validationMap.add(commandName, ValidationVector());
    371362    addResult.first->second.append(item);
    372363    if (addResult.second) {
    373         // FIXME: The function should be renamed validateCommand because it is not specific to menu items.
     364        // If we are not already awaiting validation for this command, start the asynchronous validation process.
     365        // FIXME: Theoretically, there is a race here; when we get the answer it might be old, from a previous time
     366        // we asked for the same command; there is no guarantee the answer is still valid.
     367        // FIXME: The function called here should be renamed validateCommand because it is not specific to menu items.
    374368        _data->_page->validateMenuItem(commandName);
    375369    }
    376370
    377371    // Treat as enabled until we get the result back from the web process and _setUserInterfaceItemState is called.
    378     // FIXME: This means that items will flash enabled at first, and only then disable a moment later, which is unattractive.
    379     // But returning NO here is worse, because that makes keyboard commands such as command-C fail.
     372    // FIXME <rdar://problem/8803459>: This means disabled items will flash enabled at first for a moment.
     373    // But returning NO here would be worse; that would make keyboard commands such as command-C fail.
    380374    return YES;
    381375}
    … …  
    989983        [menuItem(item) setEnabled:isEnabled];
    990984        [toolbarItem(item) setEnabled:isEnabled];
    991         // FIXME: If the user interface item is neither a menu item nor a toolbar, it will be left enabled.
    992         // It's not obvious how to fix this.
     985        // FIXME <rdar://problem/8803392>: If the item is neither a menu nor toolbar item, it will be left enabled.
    993986    }
    994987}
Note: See TracChangeset for help on using the changeset viewer.