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 6ccf18d..19a3d1d 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 1b6ac11..ded1cc9 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 d521c03..f4a2f4b 100644 (file)
@@ -41,6 +41,7 @@ public:
     enum class ErrorCode {
         InvalidArgument,
         InvalidElementState,
+        InvalidSelector,
         InvalidSessionID,
         JavascriptError,
         NoSuchElement,
index f34ba89..ae3aa2c 100644 (file)
@@ -1,5 +1,28 @@
 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
         https://bugs.webkit.org/show_bug.cgi?id=174624
 
index ccaa67c..b3f2a33 100644 (file)
@@ -57,7 +57,8 @@
                 "NoJavaScriptDialog",
                 "NotImplemented",
                 "MissingParameter",
-                "InvalidParameter"
+                "InvalidParameter",
+                "InvalidSelector"
             ]
         },
         {
index 3e59893..1e99052 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 2a829f6..8ee4274 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));