Commit 50fec70c authored by darin@apple.com's avatar darin@apple.com

2010-12-23 Darin Adler <darin@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@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
parent 23d5275c
2010-12-23 Darin Adler <darin@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@apple.com>
Reviewed by Simon Fraser.
......
......@@ -715,6 +715,7 @@ __ZN7WebCore6Editor33increaseSelectionListLevelOrderedEv
__ZN7WebCore6Editor34setMarkedTextMatchesAreHighlightedEb
__ZN7WebCore6Editor35increaseSelectionListLevelUnorderedEv
__ZN7WebCore6Editor35setIgnoreCompositionSelectionChangeEb
__ZN7WebCore6Editor38commandIsSupportedFromMenuOrKeyBindingERKN3WTF6StringE
__ZN7WebCore6Editor3cutEv
__ZN7WebCore6Editor44confirmCompositionWithoutDisturbingSelectionEv
__ZN7WebCore6Editor4copyEv
......
......@@ -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);
......
......@@ -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
......
2010-12-23 Darin Adler <darin@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@webkit.org>
Reviewed by Anders Carlsson.
......@@ -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.
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment