WebDriver: handle invalid selector errors
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jul 2017 06:11:50 +0000 (06:11 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Jul 2017 06:11:50 +0000 (06:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174619

Reviewed by Brian Burg.

Source/WebDriver:

Add InvalidSelector error and handle it in case of protocol server error.

* CommandResult.cpp:
(WebDriver::CommandResult::CommandResult):
(WebDriver::CommandResult::httpStatusCode):
(WebDriver::CommandResult::errorString):
* CommandResult.h:

Source/WebKit:

We are currently handling only XPathException and only when it's an invalid expression. In the xpath case, the
spec also says "If any item in result is not an element return an error with error code invalid selector.", so
we should also handle TYPE_ERR (The expression could not be converted to return the specified type.). However,
since the spec says "or other error", I think we can simplify this and simply throw InvalidSelector inside the
catch, without checking any specific error. This is causing 14 failures in selenium tests.

§12. Element Retrieval. Step 6: If a DOMException, SyntaxError, XPathException, or other error occurs during the
execution of the element location strategy, return error invalid selector.
https://www.w3.org/TR/webdriver/#dfn-find

* UIProcess/Automation/Automation.json: Add InvalidSelector error.
* UIProcess/Automation/atoms/FindNodes.js:
(tryToFindNode): Raise InvalidSelector in case of error.
* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::WebAutomationSessionProxy::evaluateJavaScriptFunction): Handle InvalidSelector exceptions.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219652 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebDriver/ChangeLog
Source/WebDriver/CommandResult.cpp
Source/WebDriver/CommandResult.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/Automation/Automation.json
Source/WebKit/UIProcess/Automation/atoms/FindNodes.js
Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp

index 6ccf18d0585975d8ffe7cb65f56f1dfc367b53cb..19a3d1d51f05323420d5ef72a72da28ee90a0b9c 100644 (file)
@@ -1,3 +1,18 @@
+2017-07-18  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        WebDriver: handle invalid selector errors
+        https://bugs.webkit.org/show_bug.cgi?id=174619
+
+        Reviewed by Brian Burg.
+
+        Add InvalidSelector error and handle it in case of protocol server error.
+
+        * CommandResult.cpp:
+        (WebDriver::CommandResult::CommandResult):
+        (WebDriver::CommandResult::httpStatusCode):
+        (WebDriver::CommandResult::errorString):
+        * CommandResult.h:
+
 2017-07-18  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         [GTK] Fix build with Clang after r219605.
index 1b6ac11c737203e0f9913ebc221ef55046e89b3f..ded1cc984a76292a3bc9a61a436caa84d84c3194 100644 (file)
@@ -98,6 +98,8 @@ CommandResult::CommandResult(RefPtr<InspectorValue>&& result, std::optional<Erro
             m_errorCode = ErrorCode::InvalidArgument;
         else if (errorName == "InvalidElementState")
             m_errorCode = ErrorCode::InvalidElementState;
+        else if (errorName == "InvalidSelector")
+            m_errorCode = ErrorCode::InvalidSelector;
 
         break;
     }
@@ -120,6 +122,7 @@ unsigned CommandResult::httpStatusCode() const
     switch (m_errorCode.value()) {
     case ErrorCode::InvalidArgument:
     case ErrorCode::InvalidElementState:
+    case ErrorCode::InvalidSelector:
     case ErrorCode::NoSuchElement:
     case ErrorCode::NoSuchFrame:
     case ErrorCode::NoSuchWindow:
@@ -150,6 +153,8 @@ String CommandResult::errorString() const
         return ASCIILiteral("invalid argument");
     case ErrorCode::InvalidElementState:
         return ASCIILiteral("invalid element state");
+    case ErrorCode::InvalidSelector:
+        return ASCIILiteral("invalid selector");
     case ErrorCode::InvalidSessionID:
         return ASCIILiteral("invalid session id");
     case ErrorCode::JavascriptError:
index d521c03bf0a73a4c67be3ab03b0f8f0b01a2120c..f4a2f4b271b06a1ba2257148a923a3b1d1705af4 100644 (file)
@@ -41,6 +41,7 @@ public:
     enum class ErrorCode {
         InvalidArgument,
         InvalidElementState,
+        InvalidSelector,
         InvalidSessionID,
         JavascriptError,
         NoSuchElement,
index f34ba89dfc1f2316f3b7d4132e90fdf88f094d2a..ae3aa2c9bffb12493aa9fdfde3a885f10bd1d94b 100644 (file)
@@ -1,3 +1,26 @@
+2017-07-18  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        WebDriver: handle invalid selector errors
+        https://bugs.webkit.org/show_bug.cgi?id=174619
+
+        Reviewed by Brian Burg.
+
+        We are currently handling only XPathException and only when it's an invalid expression. In the xpath case, the
+        spec also says "If any item in result is not an element return an error with error code invalid selector.", so
+        we should also handle TYPE_ERR (The expression could not be converted to return the specified type.). However,
+        since the spec says "or other error", I think we can simplify this and simply throw InvalidSelector inside the
+        catch, without checking any specific error. This is causing 14 failures in selenium tests.
+
+        §12. Element Retrieval. Step 6: If a DOMException, SyntaxError, XPathException, or other error occurs during the
+        execution of the element location strategy, return error invalid selector.
+        https://www.w3.org/TR/webdriver/#dfn-find
+
+        * UIProcess/Automation/Automation.json: Add InvalidSelector error.
+        * UIProcess/Automation/atoms/FindNodes.js:
+        (tryToFindNode): Raise InvalidSelector in case of error.
+        * WebProcess/Automation/WebAutomationSessionProxy.cpp:
+        (WebKit::WebAutomationSessionProxy::evaluateJavaScriptFunction): Handle InvalidSelector exceptions.
+
 2017-07-18  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Web Automation: error details not passed to DidEvaluateJavaScriptFunction message when callback was not called before page unload
index ccaa67ce0d386cba50a97546228cb3d2a26ee8ad..b3f2a3313c1927d04e112fedb2bac46e53c3e205 100644 (file)
@@ -57,7 +57,8 @@
                 "NoJavaScriptDialog",
                 "NotImplemented",
                 "MissingParameter",
-                "InvalidParameter"
+                "InvalidParameter",
+                "InvalidSelector"
             ]
         },
         {
index 3e59893963055193c889668335b9fab33e346bcb..1e990524c4bcd752302ced63796c36f324aca161 100644 (file)
@@ -100,11 +100,10 @@ function(strategy, ancestorElement, query, firstResultOnly, timeoutDuration, cal
                 return arrayResult;
             }
         } catch (error) {
-            if (error instanceof XPathException && error.code === XPathException.INVALID_EXPRESSION_ERR)
-                return "InvalidXPathExpression";
-            // FIXME: Bad CSS can throw an error that we should report back to the endpoint. There is no
-            // special error code for that though, so we just return an empty match.
-            return firstResultOnly ? null : [];
+            // §12. Element Retrieval. Step 6: If a DOMException, SyntaxError, XPathException, or other error occurs during
+            // the execution of the element location strategy, return error invalid selector.
+            // https://www.w3.org/TR/webdriver/#dfn-find
+            throw { name: "InvalidSelector", message: error.message };
         }
     }
 
index 2a829f6a4ef2137cf0876c2924dbf753894e376a..8ee427427868fce21296a8581878a911886537c1 100644 (file)
@@ -281,6 +281,8 @@ void WebAutomationSessionProxy::evaluateJavaScriptFunction(uint64_t pageID, uint
             errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidElementState);
         else if (exceptionName->string() == "InvalidParameter")
             errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidParameter);
+        else if (exceptionName->string() == "InvalidSelector")
+            errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidSelector);
 
         JSValueRef messageValue = JSObjectGetProperty(context, const_cast<JSObjectRef>(exception), toJSString(ASCIILiteral("message")).get(), nullptr);
         exceptionMessage.adopt(JSValueToStringCopy(context, messageValue, nullptr));