Skip to content

Commit

Permalink
Reverting IE driver stale element checks to previous behavior
Browse files Browse the repository at this point in the history
In the initial refactoring to allow use of JSON objects for executing
JavaScript, the checks for whether an element is valid (attached to the
DOM and that the document is the current document) were moved from the
Element class to the IECommandExecutor class. This was not the right
approach. These checks are moved back, and a new check instituted to
validate the element as an argument.
  • Loading branch information
jimevans committed Feb 9, 2018
1 parent 72f233b commit 29f3d39
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 34 deletions.
5 changes: 5 additions & 0 deletions cpp/iedriver/Element.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ Element::Element(IHTMLElement* element, HWND containing_window_handle) {
this->containing_window_handle_ = containing_window_handle;
}

Element::Element(IHTMLElement* element, std::string element_id) {
this->element_ = element;
this->element_id_ = element_id;
}

Element::~Element(void) {
}

Expand Down
1 change: 1 addition & 0 deletions cpp/iedriver/Element.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Browser;
class Element {
public:
Element(IHTMLElement* element, HWND containing_window_handle);
Element(IHTMLElement* element, std::string element_id);
virtual ~Element(void);
Json::Value ConvertToJson(void);
std::string GetTagName(void);
Expand Down
32 changes: 1 addition & 31 deletions cpp/iedriver/IECommandExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -794,37 +794,7 @@ int IECommandExecutor::CreateNewBrowser(std::string* error_message) {
int IECommandExecutor::GetManagedElement(const std::string& element_id,
ElementHandle* element_wrapper) const {
LOG(TRACE) << "Entering IECommandExecutor::GetManagedElement";
ElementHandle candidate_wrapper;
int result = this->managed_elements_->GetManagedElement(element_id, &candidate_wrapper);
if (result != WD_SUCCESS) {
LOG(WARN) << "Unable to get managed element, element not found";
return result;
} else {
if (!candidate_wrapper->IsAttachedToDom()) {
LOG(WARN) << "Found managed element is no longer valid";
this->managed_elements_->RemoveManagedElement(element_id);
return EOBSOLETEELEMENT;
} else {
// If the element is attached to the DOM, validate that its document
// is the currently-focused document (via frames).
BrowserHandle current_browser;
this->GetCurrentBrowser(&current_browser);
CComPtr<IHTMLDocument2> focused_doc;
current_browser->GetDocument(&focused_doc);

CComPtr<IDispatch> parent_doc_dispatch;
candidate_wrapper->element()->get_document(&parent_doc_dispatch);

if (focused_doc.IsEqualObject(parent_doc_dispatch)) {
*element_wrapper = candidate_wrapper;
return WD_SUCCESS;
} else {
LOG(WARN) << "Found managed element's document is not currently focused";
}
}
}

return EOBSOLETEELEMENT;
return this->managed_elements_->GetManagedElement(element_id, element_wrapper);
}

void IECommandExecutor::AddManagedElement(IHTMLElement* element,
Expand Down
33 changes: 32 additions & 1 deletion cpp/iedriver/IECommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,38 @@ int IECommandHandler::GetElement(const IECommandExecutor& executor,
const std::string& element_id,
ElementHandle* element_wrapper) {
LOG(TRACE) << "Entering IECommandHandler::GetElement";
return executor.GetManagedElement(element_id, element_wrapper);
ElementHandle candidate_wrapper;
int result = executor.GetManagedElement(element_id, &candidate_wrapper);
if (result != WD_SUCCESS) {
LOG(WARN) << "Unable to get managed element, element not found";
return result;
} else {
if (!candidate_wrapper->IsAttachedToDom()) {
LOG(WARN) << "Found managed element is no longer valid";
IECommandExecutor& mutable_executor = const_cast<IECommandExecutor&>(executor);
mutable_executor.RemoveManagedElement(element_id);
return EOBSOLETEELEMENT;
} else {
// If the element is attached to the DOM, validate that its document
// is the currently-focused document (via frames).
BrowserHandle current_browser;
executor.GetCurrentBrowser(&current_browser);
CComPtr<IHTMLDocument2> focused_doc;
current_browser->GetDocument(&focused_doc);

CComPtr<IDispatch> parent_doc_dispatch;
candidate_wrapper->element()->get_document(&parent_doc_dispatch);

if (focused_doc.IsEqualObject(parent_doc_dispatch)) {
*element_wrapper = candidate_wrapper;
return WD_SUCCESS;
} else {
LOG(WARN) << "Found managed element's document is not currently focused";
}
}
}

return EOBSOLETEELEMENT;
}

Json::Value IECommandHandler::RecreateJsonParameterObject(const ParametersMap& command_parameters) {
Expand Down
19 changes: 17 additions & 2 deletions cpp/iedriver/Script.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,10 +674,25 @@ int Script::AddArgument(HWND element_repository_handle, const Json::Value& arg)
if (arg.isMember(element_marker_property_name)) {
ElementInfo info;
info.element_id = arg[element_marker_property_name].asString();
status_code = static_cast<int>(::SendMessage(element_repository_handle, WD_GET_MANAGED_ELEMENT, NULL, reinterpret_cast<LPARAM>(&info)));
status_code = static_cast<int>(::SendMessage(element_repository_handle,
WD_GET_MANAGED_ELEMENT,
NULL,
reinterpret_cast<LPARAM>(&info)));

if (status_code == WD_SUCCESS) {
this->AddArgument(info.element);
ElementHandle wrapped_element(new Element(info.element, info.element_id));
bool is_element_valid = wrapped_element->IsAttachedToDom();
if (is_element_valid) {
CComPtr<IDispatch> parent_doc_dispatch;
wrapped_element->element()->get_document(&parent_doc_dispatch);
is_element_valid = this->script_engine_host_.IsEqualObject(parent_doc_dispatch);
}

if (is_element_valid) {
this->AddArgument(info.element);
} else {
status_code = EOBSOLETEELEMENT;
}
}
} else {
status_code = this->WalkObject(element_repository_handle, arg);
Expand Down

0 comments on commit 29f3d39

Please sign in to comment.