From c86d229ddf28d7e0b789ce136ebe5ed0e1bdc1b4 Mon Sep 17 00:00:00 2001 From: Alexei Barantsev Date: Fri, 8 Dec 2017 08:54:23 +0300 Subject: [PATCH] Fixing legacy Firefox driver (and atoms) to throw proper exception on an attempt to return recursive object. --- .../selenium/ExecutingJavascriptTest.java | 2 - javascript/atoms/inject.js | 2 +- javascript/firefox-driver/js/firefoxDriver.js | 6 +- javascript/firefox-driver/js/utils.js | 135 ++++++++++-------- 4 files changed, 80 insertions(+), 65 deletions(-) diff --git a/java/client/test/org/openqa/selenium/ExecutingJavascriptTest.java b/java/client/test/org/openqa/selenium/ExecutingJavascriptTest.java index 7e96fa2f6214a..3a075af53fcf8 100644 --- a/java/client/test/org/openqa/selenium/ExecutingJavascriptTest.java +++ b/java/client/test/org/openqa/selenium/ExecutingJavascriptTest.java @@ -32,7 +32,6 @@ import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; import static org.openqa.selenium.testing.Driver.CHROME; -import static org.openqa.selenium.testing.Driver.FIREFOX; import static org.openqa.selenium.testing.Driver.HTMLUNIT; import static org.openqa.selenium.testing.Driver.IE; import static org.openqa.selenium.testing.Driver.MARIONETTE; @@ -566,7 +565,6 @@ public void shouldHandleObjectThatThatHaveToJSONMethod() { @Ignore(CHROME) @Ignore(value = IE, issue = "540") @Ignore(SAFARI) - @Ignore(value = FIREFOX, issue = "540") @Ignore(HTMLUNIT) public void shouldHandleRecursiveStructures() { driver.get(pages.simpleTestPage); diff --git a/javascript/atoms/inject.js b/javascript/atoms/inject.js index afe2c9552a5e3..06075d9ecc208 100644 --- a/javascript/atoms/inject.js +++ b/javascript/atoms/inject.js @@ -121,7 +121,7 @@ bot.inject.wrapValue = function(value) { // a ton of compiler warnings. value = /**@type {!Object}*/ (value); if (seen.indexOf(value) >= 0) { - throw new bot.Error(bot.ErrorCode.UNKNOWN_ERROR, + throw new bot.Error(bot.ErrorCode.JAVASCRIPT_ERROR, 'Recursive object cannot be transferred'); } diff --git a/javascript/firefox-driver/js/firefoxDriver.js b/javascript/firefox-driver/js/firefoxDriver.js index 0a08271abebf8..ccb1914748adb 100644 --- a/javascript/firefox-driver/js/firefoxDriver.js +++ b/javascript/firefox-driver/js/firefoxDriver.js @@ -306,7 +306,11 @@ function injectAndExecuteScript(respond, parameters, isAsync, timer) { } var result = unwrappedDoc['__webdriver_evaluate']['result']; - respond.value = Utils.wrapResult(result, doc); + try { + respond.value = Utils.wrapResult(result, doc); + } catch (e) { + respond.sendError(new WebDriverError(bot.ErrorCode.JAVASCRIPT_ERROR, e)); + } respond.status = unwrappedDoc['__webdriver_evaluate']['code']; // I have no idea why this started happening. Roll on m-day diff --git a/javascript/firefox-driver/js/utils.js b/javascript/firefox-driver/js/utils.js index cd738a0835a14..8e444aa42340f 100644 --- a/javascript/firefox-driver/js/utils.js +++ b/javascript/firefox-driver/js/utils.js @@ -822,83 +822,96 @@ Utils.unwrapParameters = function(wrappedParameters, doc) { Utils.wrapResult = function(result, doc) { - result = fxdriver.moz.unwrap(result); + var _wrap = function(result, doc, seen) { + result = fxdriver.moz.unwrap(result); - // Sophisticated. - switch (typeof result) { - case 'string': - case 'number': - case 'boolean': - return result; - - case 'function': - return result.toString(); + // Sophisticated. + switch (typeof result) { + case 'string': + case 'number': + case 'boolean': + return result; - case 'undefined': - return null; + case 'function': + return result.toString(); - case 'object': - if (result == null) { + case 'undefined': return null; - } - // There's got to be a more intelligent way of detecting this. - if (result.nodeType == 1 && result['tagName']) { - return {'ELEMENT': Utils.addToKnownElements(result)}; - } + case 'object': + if (result == null) { + return null; + } - if (typeof result.getMonth === 'function') { - return result.toJSON(); - } + if (seen.indexOf(result) >= 0) { + throw new bot.Error(bot.ErrorCode.JAVASCRIPT_ERROR, + 'Recursive object cannot be transferred'); + } - if (typeof result.length === 'number' && - !(result.propertyIsEnumerable('length'))) { - var array = []; - for (var i = 0; i < result.length; i++) { - array.push(Utils.wrapResult(result[i], doc)); + // There's got to be a more intelligent way of detecting this. + if (result.nodeType == 1 && result['tagName']) { + return {'ELEMENT': Utils.addToKnownElements(result)}; } - return array; - } - // Document. Grab the document element. - if (result.nodeType == 9) { - return Utils.wrapResult(result.documentElement); - } + if (typeof result.getMonth === 'function') { + return result.toJSON(); + } - try { - var nodeList = result.QueryInterface(CI.nsIDOMNodeList); - var array = []; - for (var i = 0; i < nodeList.length; i++) { - array.push(Utils.wrapResult(result.item(i), doc)); + seen.push(result); + + if (typeof result.length === 'number' && + !(result.propertyIsEnumerable('length'))) { + var array = []; + for (var i = 0; i < result.length; i++) { + array.push(_wrap(result[i], doc, seen)); + } + return array; + } + + // Document. Grab the document element. + if (result.nodeType == 9) { + return _wrap(result.documentElement, doc, seen); } - return array; - } catch (ignored) { - goog.log.warning(Utils.LOG_, 'Error wrapping NodeList', ignored); - } - try { - // There's got to be a better way, but 'result instanceof Error' returns false - if (Object.getPrototypeOf(result) != null && goog.string.endsWith(Object.getPrototypeOf(result).toString(), 'Error')) { - try { - return fxdriver.error.toJSON(result); - } catch (ignored2) { - goog.log.info(Utils.LOG_, 'Error', ignored2); - return result.toString(); + var nodeList; + try { + nodeList = result.QueryInterface(CI.nsIDOMNodeList); + } catch (ignored) { + } + if (nodeList) { + var array = []; + for (var i = 0; i < nodeList.length; i++) { + array.push(_wrap(result.item(i), doc, seen)); } + return array; } - } catch (ignored) { - goog.log.info(Utils.LOG_, 'Error', ignored); - } - var convertedObj = {}; - for (var prop in result) { - convertedObj[prop] = Utils.wrapResult(result[prop], doc); - } - return convertedObj; + try { + // There's got to be a better way, but 'result instanceof Error' returns false + if (Object.getPrototypeOf(result) != null && goog.string.endsWith( + Object.getPrototypeOf(result).toString(), 'Error')) { + try { + return fxdriver.error.toJSON(result); + } catch (ignored2) { + goog.log.info(Utils.LOG_, 'Error', ignored2); + return result.toString(); + } + } + } catch (ignored) { + goog.log.info(Utils.LOG_, 'Error', ignored); + } - default: - return result; - } + var convertedObj = {}; + for (var prop in result) { + convertedObj[prop] = _wrap(result[prop], doc, seen); + } + return convertedObj; + + default: + return result; + } + }; + return _wrap(result, doc, []); };