[SCM] WebKit Debian packaging branch, debian/experimental, updated. debian/1.3.8-1-1049-g2e11a8e

darin at apple.com darin at apple.com
Fri Jan 21 14:38:51 UTC 2011


The following commit has been merged in the debian/experimental branch:
commit 50fec70ceecb05cec925e05edadc826344f828a1
Author: darin at apple.com <darin at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Thu Dec 23 22:20:43 2010 +0000

    2010-12-23  Darin Adler  <darin at apple.com>
    
            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 at apple.com>
    
            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.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@74580 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index ddf0be1..e168e76 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2010-12-23  Darin Adler  <darin at apple.com>
+
+        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  Matthew Delaney  <mdelaney at apple.com>
 
         Reviewed by Simon Fraser.
diff --git a/WebCore/WebCore.exp.in b/WebCore/WebCore.exp.in
index 65df2a3..e959972 100644
--- a/WebCore/WebCore.exp.in
+++ b/WebCore/WebCore.exp.in
@@ -715,6 +715,7 @@ __ZN7WebCore6Editor33increaseSelectionListLevelOrderedEv
 __ZN7WebCore6Editor34setMarkedTextMatchesAreHighlightedEb
 __ZN7WebCore6Editor35increaseSelectionListLevelUnorderedEv
 __ZN7WebCore6Editor35setIgnoreCompositionSelectionChangeEb
+__ZN7WebCore6Editor38commandIsSupportedFromMenuOrKeyBindingERKN3WTF6StringE
 __ZN7WebCore6Editor3cutEv
 __ZN7WebCore6Editor44confirmCompositionWithoutDisturbingSelectionEv
 __ZN7WebCore6Editor4copyEv
diff --git a/WebCore/editing/Editor.h b/WebCore/editing/Editor.h
index 1be072c..2e61ce6 100644
--- a/WebCore/editing/Editor.h
+++ b/WebCore/editing/Editor.h
@@ -175,7 +175,7 @@ public:
     class Command {
     public:
         Command();
-        Command(PassRefPtr<Frame>, const EditorInternalCommand*, EditorCommandSource);
+        Command(const EditorInternalCommand*, EditorCommandSource, PassRefPtr<Frame>);
 
         bool execute(const String& parameter = String(), Event* triggeringEvent = 0) const;
         bool execute(Event* triggeringEvent) const;
@@ -189,12 +189,13 @@ public:
         bool isTextInsertion() const;
 
     private:
-        RefPtr<Frame> m_frame;
         const EditorInternalCommand* m_command;
         EditorCommandSource m_source;
+        RefPtr<Frame> m_frame;
     };
-    Command command(const String& commandName); // Default is CommandFromMenuOrKeyBinding.
+    Command command(const String& commandName); // Command source is CommandFromMenuOrKeyBinding.
     Command command(const String& commandName, EditorCommandSource);
+    static bool commandIsSupportedFromMenuOrKeyBinding(const String& commandName); // Works without a frame.
 
     bool insertText(const String&, Event* triggeringEvent);
     bool insertTextWithoutSendingTextEvent(const String&, bool selectInsertedText, Event* triggeringEvent);
diff --git a/WebCore/editing/EditorCommand.cpp b/WebCore/editing/EditorCommand.cpp
index 8a6b931..e460602 100644
--- a/WebCore/editing/EditorCommand.cpp
+++ b/WebCore/editing/EditorCommand.cpp
@@ -66,7 +66,7 @@ using namespace HTMLNames;
 class EditorInternalCommand {
 public:
     bool (*execute)(Frame*, Event*, EditorCommandSource, const String&);
-    bool (*isSupported)(Frame*, EditorCommandSource);
+    bool (*isSupportedFromDOM)(Frame*);
     bool (*isEnabled)(Frame*, Event*, EditorCommandSource);
     TriState (*state)(Frame*, Event*);
     String (*value)(Frame*, Event*);
@@ -1088,52 +1088,27 @@ static bool executeCancelOperation(Frame* frame, Event*, EditorCommandSource, co
 
 // Supported functions
 
-static bool supported(Frame*, EditorCommandSource)
+static bool supported(Frame*)
 {
     return true;
 }
 
-static bool supportedFromMenuOrKeyBinding(Frame*, EditorCommandSource source)
+static bool supportedFromMenuOrKeyBinding(Frame*)
 {
-    return source == CommandFromMenuOrKeyBinding;
-}
-
-static bool supportedCopyCut(Frame* frame, EditorCommandSource source)
-{
-    switch (source) {
-    case CommandFromMenuOrKeyBinding:
-        return true;
-    case CommandFromDOM:
-    case CommandFromDOMWithUserInterface: {
-        Settings* settings = frame ? frame->settings() : 0;
-        return settings && settings->javaScriptCanAccessClipboard();
-    }
-    }
-    ASSERT_NOT_REACHED();
     return false;
 }
 
-static bool supportedPaste(Frame* frame, EditorCommandSource source)
+static bool supportedCopyCut(Frame* frame)
 {
-    switch (source) {
-    case CommandFromMenuOrKeyBinding:
-        return true;
-    case CommandFromDOM:
-    case CommandFromDOMWithUserInterface: {
-        Settings* settings = frame ? frame->settings() : 0;
-        return settings && (settings->javaScriptCanAccessClipboard() ? settings->isDOMPasteAllowed() : 0);
-    }
-    }
-    ASSERT_NOT_REACHED();
-    return false;
+    Settings* settings = frame ? frame->settings() : 0;
+    return settings && settings->javaScriptCanAccessClipboard();
 }
 
-#if SUPPORT_AUTOCORRECTION_PANEL
-static bool supportedDismissCorrectionPanel(Frame* frame, EditorCommandSource source)
+static bool supportedPaste(Frame* frame)
 {
-    return supportedFromMenuOrKeyBinding(frame, source) && frame->editor()->isShowingCorrectionPanel();
+    Settings* settings = frame ? frame->settings() : 0;
+    return settings && (settings->javaScriptCanAccessClipboard() ? settings->isDOMPasteAllowed() : 0);
 }
-#endif
 
 // Enabled functions
 
@@ -1249,6 +1224,13 @@ static bool enabledUndo(Frame* frame, Event*, EditorCommandSource)
     return frame->editor()->canUndo();
 }
 
+#if SUPPORT_AUTOCORRECTION_PANEL
+static bool enabledDismissCorrectionPanel(Frame* frame, EditorCommandSource source)
+{
+    return frame->editor()->isShowingCorrectionPanel();
+}
+#endif
+
 // State functions
 
 static TriState stateNone(Frame*, Event*)
@@ -1506,9 +1488,6 @@ static const CommandMap& createCommandMap()
         { "Subscript", { executeSubscript, supported, enabledInRichlyEditableText, stateSubscript, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
         { "Superscript", { executeSuperscript, supported, enabledInRichlyEditableText, stateSuperscript, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
         { "SwapWithMark", { executeSwapWithMark, supportedFromMenuOrKeyBinding, enabledVisibleSelectionAndMark, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
-#if PLATFORM(MAC)
-        { "TakeFindStringFromSelection", { executeTakeFindStringFromSelection, supportedFromMenuOrKeyBinding, enabledTakeFindStringFromSelection, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
-#endif
         { "ToggleBold", { executeToggleBold, supportedFromMenuOrKeyBinding, enabledInRichlyEditableText, stateBold, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
         { "ToggleItalic", { executeToggleItalic, supportedFromMenuOrKeyBinding, enabledInRichlyEditableText, stateItalic, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
         { "ToggleUnderline", { executeUnderline, supportedFromMenuOrKeyBinding, enabledInRichlyEditableText, stateUnderline, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
@@ -1520,8 +1499,13 @@ static const CommandMap& createCommandMap()
         { "Unselect", { executeUnselect, supported, enabledVisibleSelection, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
         { "Yank", { executeYank, supportedFromMenuOrKeyBinding, enabledInEditableText, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
         { "YankAndSelect", { executeYankAndSelect, supportedFromMenuOrKeyBinding, enabledInEditableText, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
+
+#if PLATFORM(MAC)
+        { "TakeFindStringFromSelection", { executeTakeFindStringFromSelection, supportedFromMenuOrKeyBinding, enabledTakeFindStringFromSelection, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
+#endif
+
 #if SUPPORT_AUTOCORRECTION_PANEL
-        { "CancelOperation", { executeCancelOperation, supportedDismissCorrectionPanel, enabledInEditableText, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
+        { "CancelOperation", { executeCancelOperation, supportedFromMenuOrKeyBinding, enabledDismissCorrectionPanel, stateNone, valueNull, notTextInsertion, doNotAllowExecutionWhenDisabled } },
 #endif
     };
 
@@ -1582,34 +1566,42 @@ static const CommandMap& createCommandMap()
     return commandMap;
 }
 
+static const EditorInternalCommand* internalCommand(const String& commandName)
+{
+    static const CommandMap& commandMap = createCommandMap();
+    return commandMap.get(commandName);
+}
+
 Editor::Command Editor::command(const String& commandName)
 {
-    return command(commandName, CommandFromMenuOrKeyBinding);
+    return Command(internalCommand(commandName), CommandFromMenuOrKeyBinding, m_frame);
 }
 
 Editor::Command Editor::command(const String& commandName, EditorCommandSource source)
 {
-    if (commandName.isEmpty())
-        return Command();
+    return Command(internalCommand(commandName), source, m_frame);
+}
 
-    static const CommandMap& commandMap = createCommandMap();
-    const EditorInternalCommand* internalCommand = commandMap.get(commandName);
-    return internalCommand ? Command(m_frame, internalCommand, source) : Command();
+bool Editor::commandIsSupportedFromMenuOrKeyBinding(const String& commandName)
+{
+    return internalCommand(commandName);
 }
 
 Editor::Command::Command()
     : m_command(0)
-    , m_source()
 {
 }
 
-Editor::Command::Command(PassRefPtr<Frame> frame, const EditorInternalCommand* command, EditorCommandSource source)
-    : m_frame(frame)
-    , m_command(command)
+Editor::Command::Command(const EditorInternalCommand* command, EditorCommandSource source, PassRefPtr<Frame> frame)
+    : m_command(command)
     , m_source(source)
+    , m_frame(command ? frame : 0)
 {
-    ASSERT(m_frame);
-    ASSERT(m_command);
+    // Use separate assertions so we can tell which bad thing happened.
+    if (!command)
+        ASSERT(!m_frame);
+    else
+        ASSERT(m_frame);
 }
 
 bool Editor::Command::execute(const String& parameter, Event* triggeringEvent) const
@@ -1630,7 +1622,17 @@ bool Editor::Command::execute(Event* triggeringEvent) const
 
 bool Editor::Command::isSupported() const
 {
-    return m_command && m_command->isSupported(m_frame.get(), m_source);
+    if (!m_command)
+        return false;
+    switch (m_source) {
+    case CommandFromMenuOrKeyBinding:
+        return true;
+    case CommandFromDOM:
+    case CommandFromDOMWithUserInterface:
+        return m_command->isSupportedFromDOM(m_frame.get());
+    }
+    ASSERT_NOT_REACHED();
+    return false;
 }
 
 bool Editor::Command::isEnabled(Event* triggeringEvent) const
diff --git a/WebKit2/ChangeLog b/WebKit2/ChangeLog
index 4a6e6ec..c96a4df 100644
--- a/WebKit2/ChangeLog
+++ b/WebKit2/ChangeLog
@@ -1,3 +1,17 @@
+2010-12-23  Darin Adler  <darin at apple.com>
+
+        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.
+
 2010-12-23  Sam Weinig  <sam at webkit.org>
 
         Reviewed by Anders Carlsson.
diff --git a/WebKit2/UIProcess/API/mac/WKView.mm b/WebKit2/UIProcess/API/mac/WKView.mm
index 7b5c49a..e36fea4 100644
--- a/WebKit2/UIProcess/API/mac/WKView.mm
+++ b/WebKit2/UIProcess/API/mac/WKView.mm
@@ -335,11 +335,6 @@ static NSToolbarItem *toolbarItem(id <NSValidatedUserInterfaceItem> item)
 {
     SEL action = [item action];
 
-    // FIXME: This is only needed to work around the fact that we don't return YES for
-    // selectors that are not editing commands (see below). If that's fixed, this can be removed.
-    if (action == @selector(startSpeaking:))
-        return YES;
-
     if (action == @selector(stopSpeaking:))
         return [NSApp isSpeaking];
 
@@ -356,27 +351,26 @@ static NSToolbarItem *toolbarItem(id <NSValidatedUserInterfaceItem> item)
         return YES;
     }
 
-    // FIXME: We should return YES here for selectors that are not editing commands.
-    // But at the moment there is no way to find out if a selector is an editing command or not
-    // in the UI process. So for now we assume any selectors not handled above are editing commands.
-
+    // Next, handle editor commands. Start by returning YES for anything that is not an editor command.
+    // Returning YES is the default thing to do in an AppKit validate method for any selector that is not recognized.
     String commandName = commandNameForSelector([item action]);
-    if (commandName.isEmpty())
+    if (!Editor::commandIsSupportedFromMenuOrKeyBinding(commandName))
         return YES;
 
-    // Add this menu item to the vector of items for a given command that are awaiting validation.
-    // If the item is the first to be added, then call validateMenuItem to start the asynchronous
-    // validation process.
+    // Add this item to the vector of items for a given command that are awaiting validation.
     pair<ValidationMap::iterator, bool> addResult = _data->_validationMap.add(commandName, ValidationVector());
     addResult.first->second.append(item);
     if (addResult.second) {
-        // FIXME: The function should be renamed validateCommand because it is not specific to menu items.
+        // If we are not already awaiting validation for this command, start the asynchronous validation process.
+        // FIXME: Theoretically, there is a race here; when we get the answer it might be old, from a previous time
+        // we asked for the same command; there is no guarantee the answer is still valid.
+        // FIXME: The function called here should be renamed validateCommand because it is not specific to menu items.
         _data->_page->validateMenuItem(commandName);
     }
 
     // Treat as enabled until we get the result back from the web process and _setUserInterfaceItemState is called.
-    // FIXME: This means that items will flash enabled at first, and only then disable a moment later, which is unattractive.
-    // But returning NO here is worse, because that makes keyboard commands such as command-C fail.
+    // FIXME <rdar://problem/8803459>: This means disabled items will flash enabled at first for a moment.
+    // But returning NO here would be worse; that would make keyboard commands such as command-C fail.
     return YES;
 }
 
@@ -988,8 +982,7 @@ static bool isViewVisible(NSView *view)
         [menuItem(item) setState:newState];
         [menuItem(item) setEnabled:isEnabled];
         [toolbarItem(item) setEnabled:isEnabled];
-        // FIXME: If the user interface item is neither a menu item nor a toolbar, it will be left enabled.
-        // It's not obvious how to fix this.
+        // FIXME <rdar://problem/8803392>: If the item is neither a menu nor toolbar item, it will be left enabled.
     }
 }
 

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list