196847 2019-04-11 19:48:51 -0700 WebDriver: evaluating javascript shouldn't fail if a dialog is shown 2019-05-07 03:59:57 -0700 1 1 1 Unclassified WebKit WebDriver WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=197655 InRadar P2 Normal --- 1 hi hi bburg cgarcia commit-queue hi webkit-bug-importer oldest_to_newest 1526606 0 hi 2019-04-11 19:48:51 -0700 We currently send an "unexpected alert open error" if a dialog is shown while evaluating javascript, but we really should be silently failing with no data. <https://github.com/w3c/webdriver/issues/1253> 1526607 1 hi 2019-04-11 19:49:01 -0700 <rdar://problem/49609396> 1526608 2 367279 hi 2019-04-11 19:50:01 -0700 Created attachment 367279 Patch 1526892 3 367279 bburg 2019-04-12 13:20:09 -0700 Comment on attachment 367279 Patch r=me 1526899 4 367279 commit-queue 2019-04-12 13:47:01 -0700 Comment on attachment 367279 Patch Clearing flags on attachment: 367279 Committed r244230: <https://trac.webkit.org/changeset/244230> 1526900 5 commit-queue 2019-04-12 13:47:02 -0700 All reviewed patches have been landed. Closing bug. 1533833 6 367279 cgarcia 2019-05-07 01:58:37 -0700 Comment on attachment 367279 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367279&action=review > Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:657 > - callback->sendFailure(unexpectedAlertOpenError); > + callback->sendSuccess(emptyString()); This is not correct, because emptyString() is not a valid JSON serialized string. JSON::Value::parseJSON() will fail when trying to handle this result. I'm not even sure this is the right fix, because evaluateJavaScriptFunction message is used for other commands than executeString and actions. We handle the executeScript case in WebDriver itself, considering a success when unexpected alert open error is received. We could do the same for the actions command. This commit has caused all user_prompts test to fail, because create_dialog() fixture always fails. It executes a script to create the dialog itself (ignoring NoSuchAlertException, indeed), that is always failing now because the response from evaluateJavaScriptFunction is not a valid JSON serialized string. So, we have two ways to fix this: 1) Roll out this patch and handle unexpected alert open in the driver for executeScript and actions commands 2) Keep handling it here, but using "null" instead of emptyString(). This is what we do in the web process when the result of the script can't be json stringfied. (return JSON.stringify(original, (key, value) => this._replaceJSONValue(key, value)) || "null";) 367279 2019-04-11 19:50:01 -0700 2019-04-12 13:47:01 -0700 Patch bug-196847-20190411195001.patch text/plain 1752 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9DaGFu Z2VMb2cKaW5kZXggMTU3ZDNhMDRjMTI2NGNkNTJiNGVkYmIzZDRlNmU0YzkzMGMzNDdjMS4uZjgz Y2QwZjI4YzQyNmE2NGIwYmM2NGE1OWE3MzFhMWIxNzFkZTM5MSAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdC9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9DaGFuZ2VMb2cKQEAgLTEsMyAr MSwxNCBAQAorMjAxOS0wNC0xMSAgRGV2aW4gUm91c3NvICA8ZHJvdXNzb0BhcHBsZS5jb20+CisK KyAgICAgICAgV2ViRHJpdmVyOiBldmFsdWF0aW5nIGphdmFzY3JpcHQgc2hvdWxkbid0IGZhaWwg aWYgYSBkaWFsb2cgaXMgc2hvd24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTE5Njg0NworICAgICAgICA8cmRhcjovL3Byb2JsZW0vNDk2MDkzOTY+CisK KyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBVSVByb2Nl c3MvQXV0b21hdGlvbi9XZWJBdXRvbWF0aW9uU2Vzc2lvbi5jcHA6CisgICAgICAgIChXZWJLaXQ6 OldlYkF1dG9tYXRpb25TZXNzaW9uOjp3aWxsU2hvd0phdmFTY3JpcHREaWFsb2cpOgorCiAyMDE5 LTA0LTExICBTYWlkIEFib3UtSGFsbGF3YSAgPHNhYm91aGFsbGF3YUBhcHBsZS5jb20+CiAKICAg ICAgICAgUkVHUkVTU0lPTiAocjI0NDE4Mik6IFJlbW90ZUxheWVyVHJlZURyYXdpbmdBcmVhOjpm bHVzaExheWVycygpIHNob3VsZCBub3QgYmUgcmVlbnRyYW50CmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViS2l0L1VJUHJvY2Vzcy9BdXRvbWF0aW9uL1dlYkF1dG9tYXRpb25TZXNzaW9uLmNwcCBiL1Nv dXJjZS9XZWJLaXQvVUlQcm9jZXNzL0F1dG9tYXRpb24vV2ViQXV0b21hdGlvblNlc3Npb24uY3Bw CmluZGV4IDU5NDIxZmQ4ODFmZDEzODZiMzMwYTJhZmU1OTUzMTAyZWEyYzVhZGQuLmI5NWE2NWZi ZjZiN2E2MGU5NTE4ODlmNjA5YTg5OGMzZTVhYTkwYWEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJL aXQvVUlQcm9jZXNzL0F1dG9tYXRpb24vV2ViQXV0b21hdGlvblNlc3Npb24uY3BwCisrKyBiL1Nv dXJjZS9XZWJLaXQvVUlQcm9jZXNzL0F1dG9tYXRpb24vV2ViQXV0b21hdGlvblNlc3Npb24uY3Bw CkBAIC02NTIsMTAgKzY1Miw5IEBAIHZvaWQgV2ViQXV0b21hdGlvblNlc3Npb246OndpbGxTaG93 SmF2YVNjcmlwdERpYWxvZyhXZWJQYWdlUHJveHkmIHBhZ2UpCiAgICAgICAgIH0KIAogICAgICAg ICBpZiAoIW1fZXZhbHVhdGVKYXZhU2NyaXB0RnVuY3Rpb25DYWxsYmFja3MuaXNFbXB0eSgpKSB7 Ci0gICAgICAgICAgICBJbnNwZWN0b3I6OkVycm9yU3RyaW5nIHVuZXhwZWN0ZWRBbGVydE9wZW5F cnJvciA9IFNUUklOR19GT1JfUFJFREVGSU5FRF9FUlJPUl9OQU1FKFVuZXhwZWN0ZWRBbGVydE9w ZW4pOwogICAgICAgICAgICAgZm9yIChhdXRvIGtleSA6IGNvcHlUb1ZlY3RvcihtX2V2YWx1YXRl SmF2YVNjcmlwdEZ1bmN0aW9uQ2FsbGJhY2tzLmtleXMoKSkpIHsKICAgICAgICAgICAgICAgICBh dXRvIGNhbGxiYWNrID0gbV9ldmFsdWF0ZUphdmFTY3JpcHRGdW5jdGlvbkNhbGxiYWNrcy50YWtl KGtleSk7Ci0gICAgICAgICAgICAgICAgY2FsbGJhY2stPnNlbmRGYWlsdXJlKHVuZXhwZWN0ZWRB bGVydE9wZW5FcnJvcik7CisgICAgICAgICAgICAgICAgY2FsbGJhY2stPnNlbmRTdWNjZXNzKGVt cHR5U3RyaW5nKCkpOwogICAgICAgICAgICAgfQogICAgICAgICB9CiAK