Skip to content

Commit

Permalink
Removing isFocusable atom from sendKeys command in IE
Browse files Browse the repository at this point in the history
The isFocusable atom does not provide the correct information as to
whether an element is focusable according to the W3C and WHATWG DOM
specifications. This change removes the use of that atom, and relies
instead on the abililty to set focus to the element before sending
the keystrokes.
  • Loading branch information
jimevans committed Dec 28, 2018
1 parent 80186ee commit dfba236
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 30 deletions.
18 changes: 11 additions & 7 deletions cpp/iedriver/CommandHandlers/SendKeysCommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,11 @@ void SendKeysCommandHandler::ExecuteInternal(
if (!this->VerifyPageHasFocus(browser_wrapper)) {
LOG(WARN) << "HTML rendering pane does not have the focus. Keystrokes may go to an unexpected UI element.";
}
if (!this->WaitUntilElementFocused(element_wrapper->element())) {
LOG(WARN) << "Specified element is not the active element. Keystrokes may go to an unexpected DOM element.";
if (!this->WaitUntilElementFocused(element_wrapper)) {
error_description = "Element cannot be interacted with via the keyboard because it is not focusable";
response->SetErrorResponse(ERROR_ELEMENT_NOT_INTERACTABLE,
error_description);
return;
}

Json::Value actions = this->CreateActionSequencePayload(executor, &keys);
Expand Down Expand Up @@ -174,10 +177,6 @@ bool SendKeysCommandHandler::IsElementInteractable(ElementHandle element_wrapper
return false;
}

if (!element_wrapper->IsFocusable()) {
*error_description = "Element cannot be interacted with via the keyboard because it is not focusable";
return false;
}
return true;
}

Expand Down Expand Up @@ -958,8 +957,13 @@ bool SendKeysCommandHandler::VerifyPageHasFocus(BrowserHandle browser_wrapper) {
return info.hwndFocus == browser_pane_window_handle;
}

bool SendKeysCommandHandler::WaitUntilElementFocused(IHTMLElement* element) {
bool SendKeysCommandHandler::WaitUntilElementFocused(ElementHandle element_wrapper) {
if (element_wrapper->IsFocusable()) {
return true;
}

// Check we have focused the element.
CComPtr<IHTMLElement> element = element_wrapper->element();
bool has_focus = false;
CComPtr<IDispatch> dispatch;
element->get_document(&dispatch);
Expand Down
2 changes: 1 addition & 1 deletion cpp/iedriver/CommandHandlers/SendKeysCommandHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ class SendKeysCommandHandler : public IECommandHandler {
bool HasMultipleAttribute(ElementHandle element_wrapper);
bool IsElementInteractable(ElementHandle element_wrapper,
std::string* error_description);
bool WaitUntilElementFocused(ElementHandle element_wrapper);
bool VerifyPageHasFocus(BrowserHandle browser_wrapper);
bool WaitUntilElementFocused(IHTMLElement* element);
bool SetInsertionPoint(IHTMLElement* element);
bool IsContentEditable(IHTMLElement* element);
void SetElementFocus(IHTMLElement* element);
Expand Down
23 changes: 1 addition & 22 deletions cpp/iedriver/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,6 @@ bool Element::IsInteractable() {
bool Element::IsFocusable() {
LOG(TRACE) << "Entering Element::IsFocusable";

bool result = false;

CComPtr<IHTMLBodyElement> body;
HRESULT hr = this->element_->QueryInterface<IHTMLBodyElement>(&body);
if (SUCCEEDED(hr) && body) {
Expand All @@ -259,26 +257,7 @@ bool Element::IsFocusable() {
return true;
}
}

// The atom is just the definition of an anonymous
// function: "function() {...}"; Wrap it in another function so we can
// invoke it with our arguments without polluting the current namespace.
std::wstring script_source(L"(function() { return (");
script_source += atoms::asString(atoms::IS_FOCUSABLE);
script_source += L")})();";


Script script_wrapper(doc, script_source, 1);
script_wrapper.AddArgument(this->element_);
int status_code = script_wrapper.Execute();

if (status_code == WD_SUCCESS) {
result = script_wrapper.result().boolVal == VARIANT_TRUE;
} else {
LOG(WARN) << "Failed to determine is element enabled";
}

return result;
return false;
}

bool Element::IsObscured(LocationInfo* click_location,
Expand Down

0 comments on commit dfba236

Please sign in to comment.