111720 2013-03-07 06:09:08 -0800 Wrong parameters passed to canExecuteScripts in ScriptEventListener (JSC and V8) 2013-05-28 07:24:24 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 111729 1 xan.lopez webkit-unassigned abarth buildbot dglazkov fpizlo ggaren haraken japhet joenotcharles oliver rniwa webkit.review.bot xan.lopez oldest_to_newest 850023 0 xan.lopez 2013-03-07 06:09:08 -0800 Not 100% sure about this, but I think that the patch in bug #35205 (old one!) made the wrong change in a couple places for ScriptEventListener, for both JSC and V8. In particular the calls that happen in the methods used to create the event listeners say that they are about to execute JS code, but AFAICT they are not (they just create the event listener object). If this is correct I can quickly make the patch, although I'm not sure of how I could test this. 850061 1 191991 xan.lopez 2013-03-07 07:17:54 -0800 Created attachment 191991 canexecutescripts.diff For reference, this would the patch. 850071 2 191991 haraken 2013-03-07 07:29:43 -0800 Comment on attachment 191991 canexecutescripts.diff How does this patch affect actual behavior? 850231 3 joenotcharles 2013-03-07 10:18:33 -0800 I don't even remember writing the patch that this is based on anymore, but looking at the code... The only difference that the AboutToExecuteScript parameter makes is that if it's set, and canExecuteScripts returns false, FrameLoaderClient::didNotAllowScript gets called. Which could, for example, open a dialog warning the user that scripts on this page did not run. But createAttributeEventListener does not actually run a script - it just creates a listener. The script isn't run unless the listener fires. So without this patch, if you disable Javascript you could get a "Javascript was blocked" warning just on loading the page, rather than when performing the action that actually runs the script. Looking at the code, I think JSLazyEventListener::initializeJSFunction has the same problem. In JSEventListener::handleEvent, canExecuteScripts(AboutToExecuteScript) gets called before the script is executed*, so the order is: JSEventListener::handleEvent calls JSEventListener::jsFunction calls JSLazyEventListener::initializeJSFunction calls canExecuteScripts(AboutToExecuteScript) -> should be NotAboutToExecuteScript, since it's just creating and returning the script, not executing it yet calls canExecuteScripts(AboutToExecuteScript) executes the script If initializeJSFunction is ever called in a different context where the script is just held for a while before executing, this would cause didNotAllowScripts to be called prematurely. *(But only for documents, with "FIXME: Is this check needed for other contexts? Should look into that. It's possible that JSLazyEventListener always calling canExecuteScripts(AboutToExecuteScript) is hiding another bug where it's not always called in JSEventListener::handleEvent.) 850389 4 191991 webkit.review.bot 2013-03-07 12:45:10 -0800 Comment on attachment 191991 canexecutescripts.diff Attachment 191991 did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17027165 New failing tests: media/video-controls-no-scripting.html fast/frames/sandboxed-iframe-autofocus-denied.html 850656 5 191991 buildbot 2013-03-07 17:00:52 -0800 Comment on attachment 191991 canexecutescripts.diff Attachment 191991 did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16991401 New failing tests: media/video-controls-no-scripting.html fast/frames/sandboxed-iframe-autofocus-denied.html 850670 6 191991 haraken 2013-03-07 17:06:30 -0800 Comment on attachment 191991 canexecutescripts.diff This change may be correct but may not be correct. It looks like more investigation is needed. Let me r- it for now. 894276 7 xan.lopez 2013-05-28 03:02:16 -0700 At the very least we can remove the V8 stuff now, but I wonder what the JSC folks think about the patch. CCing a few people. 894348 8 fpizlo 2013-05-28 07:24:24 -0700 What bug does this fix? 191991 2013-03-07 07:17:54 -0800 2013-03-07 17:06:29 -0800 canexecutescripts.diff canexecutescripts.diff text/plain 4306 xan.lopez RnJvbSA2ZTc2MTRkODZmMzNlM2E4YjkzMTc1YTdiNjZkMzk2ZjUxNjRhYTcwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBYYW4gTG9wZXogPHhhbkBpZ2FsaWEuY29tPgpEYXRlOiBUaHUs IDcgTWFyIDIwMTMgMTY6MTI6NDYgKzAxMDAKU3ViamVjdDogW1BBVENIXSBXcm9uZyBwYXJhbWV0 ZXJzIHBhc3NlZCB0byBjYW5FeGVjdXRlU2NyaXB0cyBpbgogU2NyaXB0RXZlbnRMaXN0ZW5lciAo SlNDIGFuZCBWOCkKIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMTE3 MjAKClJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgoKUGFzcyB0aGUgY29ycmVjdCBwYXJhbWV0 ZXIgdG8gY2FuRXhlY3V0ZVNjcmlwdHMgaW4gdGhlIEpTCmJpbmRpbmdzLCB0aGVzZSBtZXRob2Rz IGRvIG5vdCBhY3R1YWxseSBleGVjdXRlIGFueSBqYXZhc2NyaXB0CmNvZGUuCgpCYXNlZCBvbiBh IHBhdGNoIGJ5IEpvZSBNYXNvbiA8am1hc29uQHJpbS5jb20+CgoqIGJpbmRpbmdzL2pzL1Njcmlw dEV2ZW50TGlzdGVuZXIuY3BwOgooV2ViQ29yZTo6Y3JlYXRlQXR0cmlidXRlRXZlbnRMaXN0ZW5l cik6CiogYmluZGluZ3MvdjgvU2NyaXB0RXZlbnRMaXN0ZW5lci5jcHA6CihXZWJDb3JlOjpjcmVh dGVBdHRyaWJ1dGVFdmVudExpc3RlbmVyKToKLS0tCiBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cg ICAgICAgICAgICAgICAgICAgICAgICAgICB8IDE4ICsrKysrKysrKysrKysrKysrKwogU291cmNl L1dlYkNvcmUvYmluZGluZ3MvanMvU2NyaXB0RXZlbnRMaXN0ZW5lci5jcHAgfCAgNCArKy0tCiBT b3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9TY3JpcHRFdmVudExpc3RlbmVyLmNwcCB8ICA0ICsr LS0KIDMgZmlsZXMgY2hhbmdlZCwgMjIgaW5zZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMoLSkKCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFu Z2VMb2cKaW5kZXggMjViZjk0NS4uNTllMGI2OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUv Q2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBA CisyMDEzLTAzLTA3ICBYYW4gTG9wZXogIDx4bG9wZXpAaWdhbGlhLmNvbT4KKworICAgICAgICBX cm9uZyBwYXJhbWV0ZXJzIHBhc3NlZCB0byBjYW5FeGVjdXRlU2NyaXB0cyBpbiBTY3JpcHRFdmVu dExpc3RlbmVyIChKU0MgYW5kIFY4KQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MTExNzIwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgUGFzcyB0aGUgY29ycmVjdCBwYXJhbWV0ZXIgdG8gY2FuRXhlY3V0ZVNj cmlwdHMgaW4gdGhlIEpTCisgICAgICAgIGJpbmRpbmdzLCB0aGVzZSBtZXRob2RzIGRvIG5vdCBh Y3R1YWxseSBleGVjdXRlIGFueSBqYXZhc2NyaXB0CisgICAgICAgIGNvZGUuCisKKyAgICAgICAg QmFzZWQgb24gYSBwYXRjaCBieSBKb2UgTWFzb24gPGptYXNvbkByaW0uY29tPgorCisgICAgICAg ICogYmluZGluZ3MvanMvU2NyaXB0RXZlbnRMaXN0ZW5lci5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpjcmVhdGVBdHRyaWJ1dGVFdmVudExpc3RlbmVyKToKKyAgICAgICAgKiBiaW5kaW5ncy92OC9T Y3JpcHRFdmVudExpc3RlbmVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmNyZWF0ZUF0dHJpYnV0 ZUV2ZW50TGlzdGVuZXIpOgorCiAyMDEzLTAzLTA3ICBTaGVyaWZmIEJvdCAgPHdlYmtpdC5yZXZp ZXcuYm90QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTQ1 MDU5LgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvU2NyaXB0RXZlbnRM aXN0ZW5lci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9qcy9TY3JpcHRFdmVudExpc3Rl bmVyLmNwcAppbmRleCA5MTU3MTI0Li42NmE1ZjIwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9iaW5kaW5ncy9qcy9TY3JpcHRFdmVudExpc3RlbmVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29y ZS9iaW5kaW5ncy9qcy9TY3JpcHRFdmVudExpc3RlbmVyLmNwcApAQCAtNjQsNyArNjQsNyBAQCBQ YXNzUmVmUHRyPEpTTGF6eUV2ZW50TGlzdGVuZXI+IGNyZWF0ZUF0dHJpYnV0ZUV2ZW50TGlzdGVu ZXIoTm9kZSogbm9kZSwgY29uc3QgUQogICAgIC8vIEZJWE1FOiBXZSBzaG91bGQgYmUgYWJsZSB0 byBwcm92aWRlIGFjY3VyYXRlIHNvdXJjZSBpbmZvcm1hdGlvbiBmb3IgZnJhbWVsZXNzIGRvY3Vt ZW50cywgdG9vIChlLmcuIGZvciBpbXBvcnRpbmcgbm9kZXMgZnJvbSBYTUxIdHRwUmVxdWVzdC5y ZXNwb25zZVhNTCkuCiAgICAgaWYgKEZyYW1lKiBmcmFtZSA9IG5vZGUtPmRvY3VtZW50KCktPmZy YW1lKCkpIHsKICAgICAgICAgU2NyaXB0Q29udHJvbGxlciogc2NyaXB0Q29udHJvbGxlciA9IGZy YW1lLT5zY3JpcHQoKTsKLSAgICAgICAgaWYgKCFzY3JpcHRDb250cm9sbGVyLT5jYW5FeGVjdXRl U2NyaXB0cyhBYm91dFRvRXhlY3V0ZVNjcmlwdCkpCisgICAgICAgIGlmICghc2NyaXB0Q29udHJv bGxlci0+Y2FuRXhlY3V0ZVNjcmlwdHMoTm90QWJvdXRUb0V4ZWN1dGVTY3JpcHQpKQogICAgICAg ICAgICAgcmV0dXJuIDA7CiAKICAgICAgICAgcG9zaXRpb24gPSBzY3JpcHRDb250cm9sbGVyLT5l dmVudEhhbmRsZXJQb3NpdGlvbigpOwpAQCAtODMsNyArODMsNyBAQCBQYXNzUmVmUHRyPEpTTGF6 eUV2ZW50TGlzdGVuZXI+IGNyZWF0ZUF0dHJpYnV0ZUV2ZW50TGlzdGVuZXIoRnJhbWUqIGZyYW1l LCBjb25zdAogICAgICAgICByZXR1cm4gMDsKIAogICAgIFNjcmlwdENvbnRyb2xsZXIqIHNjcmlw dENvbnRyb2xsZXIgPSBmcmFtZS0+c2NyaXB0KCk7Ci0gICAgaWYgKCFzY3JpcHRDb250cm9sbGVy LT5jYW5FeGVjdXRlU2NyaXB0cyhBYm91dFRvRXhlY3V0ZVNjcmlwdCkpCisgICAgaWYgKCFzY3Jp cHRDb250cm9sbGVyLT5jYW5FeGVjdXRlU2NyaXB0cyhOb3RBYm91dFRvRXhlY3V0ZVNjcmlwdCkp CiAgICAgICAgIHJldHVybiAwOwogCiAgICAgVGV4dFBvc2l0aW9uIHBvc2l0aW9uID0gc2NyaXB0 Q29udHJvbGxlci0+ZXZlbnRIYW5kbGVyUG9zaXRpb24oKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL2JpbmRpbmdzL3Y4L1NjcmlwdEV2ZW50TGlzdGVuZXIuY3BwIGIvU291cmNlL1dlYkNv cmUvYmluZGluZ3MvdjgvU2NyaXB0RXZlbnRMaXN0ZW5lci5jcHAKaW5kZXggMjFkMjQ2MC4uZjFk NDAyMCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvU2NyaXB0RXZlbnRM aXN0ZW5lci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvU2NyaXB0RXZlbnRM aXN0ZW5lci5jcHAKQEAgLTYwLDcgKzYwLDcgQEAgUGFzc1JlZlB0cjxWOExhenlFdmVudExpc3Rl bmVyPiBjcmVhdGVBdHRyaWJ1dGVFdmVudExpc3RlbmVyKE5vZGUqIG5vZGUsIGNvbnN0IFEKIAog ICAgIGlmIChGcmFtZSogZnJhbWUgPSBub2RlLT5kb2N1bWVudCgpLT5mcmFtZSgpKSB7CiAgICAg ICAgIFNjcmlwdENvbnRyb2xsZXIqIHNjcmlwdENvbnRyb2xsZXIgPSBmcmFtZS0+c2NyaXB0KCk7 Ci0gICAgICAgIGlmICghc2NyaXB0Q29udHJvbGxlci0+Y2FuRXhlY3V0ZVNjcmlwdHMoQWJvdXRU b0V4ZWN1dGVTY3JpcHQpKQorICAgICAgICBpZiAoIXNjcmlwdENvbnRyb2xsZXItPmNhbkV4ZWN1 dGVTY3JpcHRzKE5vdEFib3V0VG9FeGVjdXRlU2NyaXB0KSkKICAgICAgICAgICAgIHJldHVybiAw OwogICAgICAgICBwb3NpdGlvbiA9IHNjcmlwdENvbnRyb2xsZXItPmV2ZW50SGFuZGxlclBvc2l0 aW9uKCk7CiAgICAgICAgIHNvdXJjZVVSTCA9IG5vZGUtPmRvY3VtZW50KCktPnVybCgpLnN0cmlu ZygpOwpAQCAtNzgsNyArNzgsNyBAQCBQYXNzUmVmUHRyPFY4TGF6eUV2ZW50TGlzdGVuZXI+IGNy ZWF0ZUF0dHJpYnV0ZUV2ZW50TGlzdGVuZXIoRnJhbWUqIGZyYW1lLCBjb25zdAogICAgICAgICBy ZXR1cm4gMDsKIAogICAgIFNjcmlwdENvbnRyb2xsZXIqIHNjcmlwdENvbnRyb2xsZXIgPSBmcmFt ZS0+c2NyaXB0KCk7Ci0gICAgaWYgKCFzY3JpcHRDb250cm9sbGVyLT5jYW5FeGVjdXRlU2NyaXB0 cyhBYm91dFRvRXhlY3V0ZVNjcmlwdCkpCisgICAgaWYgKCFzY3JpcHRDb250cm9sbGVyLT5jYW5F eGVjdXRlU2NyaXB0cyhOb3RBYm91dFRvRXhlY3V0ZVNjcmlwdCkpCiAgICAgICAgIHJldHVybiAw OwogCiAgICAgVGV4dFBvc2l0aW9uIHBvc2l0aW9uID0gc2NyaXB0Q29udHJvbGxlci0+ZXZlbnRI YW5kbGVyUG9zaXRpb24oKTsKLS0gCjEuOC4xLjQKCg==