Setting an expando property on <object> stopped working
Reported by
dan.abra...@gmail.com,
Jan 9 2017
|
|||||||||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.95 Safari/537.36
Steps to reproduce the problem:
let foo = document.createElement('object');
foo.test = 5;
alert(foo.test);
What is the expected behavior?
It alerts 5.
What went wrong?
It alerts undefined.
Did this work before? Yes Definitely worked in 55.0.2883.95, maybe in more recent ones too
Does this work in other browsers? Yes
Chrome version: 57.0.2976.0 Channel: canary
OS Version: OS X 10.12.1
Flash Version: Shockwave Flash 24.0 r0
This fundamentally breaks React and probably other libraries.
https://github.com/facebook/react/issues/8718
,
Jan 10 2017
439806 (good) - 439813 (bad) https://chromium.googlesource.com/chromium/src/+log/5daea93d..0c55868e?pretty=fuller Suspecting r439812 "Better handle non-existant wrappers when intercepting writes to plugin objects"
,
Jan 10 2017
jsfiddle based on the OP's code modified to display the result on the page instead of alert: https://jsfiddle.net/wOxxOm/ghcgn3b0/embedded/result/
,
Jan 10 2017
Able to reproduce this issue on Windows-10, Mac OS 10.12 and Ubuntu 14.04 using chrome latest canary #57.0.2976.0. As per comment #2 bisect info assigning to @jochen for more updates on this issue.
,
Jan 10 2017
,
Jan 10 2017
,
Jan 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/ed43c63e26758e01631331ea820ef90baf4eb92f commit ed43c63e26758e01631331ea820ef90baf4eb92f Author: jochen <jochen@chromium.org> Date: Tue Jan 10 12:22:21 2017 Remove API check for interceptors to notify us of modifying the receiver BUG= chromium:679345 R=verwaest@chromium.org Review-Url: https://codereview.chromium.org/2622873002 Cr-Commit-Position: refs/heads/master@{#42173} [modify] https://crrev.com/ed43c63e26758e01631331ea820ef90baf4eb92f/src/objects.cc
,
Jan 10 2017
Thanks for the fix , we will verify in today's canary. If all looks good please request a merge to M56.
,
Jan 12 2017
still depends on the blink side patch (that had to wait for a day for v8 to roll)
,
Jan 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a4fe2fc28bb45f58445d5802057efdafbe7719b commit 8a4fe2fc28bb45f58445d5802057efdafbe7719b Author: jochen <jochen@chromium.org> Date: Thu Jan 12 12:46:09 2017 Don't try to correctly notify V8 from the plugin interceptor Instead, I modified V8 to just cope with this behavior BUG= chromium:679345 R=haraken@chromium.org Review-Url: https://codereview.chromium.org/2625643002 Cr-Commit-Position: refs/heads/master@{#443212} [modify] https://crrev.com/8a4fe2fc28bb45f58445d5802057efdafbe7719b/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-embedded-expected.txt [add] https://crrev.com/8a4fe2fc28bb45f58445d5802057efdafbe7719b/third_party/WebKit/LayoutTests/plugins/simple-expando.html [modify] https://crrev.com/8a4fe2fc28bb45f58445d5802057efdafbe7719b/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLPlugInElementCustom.cpp
,
Jan 12 2017
,
Jan 12 2017
[Auto-generated comment by a script] We noticed that this issue is targeted for M-56; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-56 label, otherwise remove Merge-TBD label. Thanks.
,
Jan 13 2017
,
Jan 13 2017
Your change meets the bar and is auto-approved for M56. Please go ahead and merge the CL manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), gkihumba@(cros), bustamante@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/6c7373d444966fbd4e1e4aabad0be4dc66b9eee3 commit 6c7373d444966fbd4e1e4aabad0be4dc66b9eee3 Author: Jochen Eisinger <jochen@chromium.org> Date: Fri Jan 13 13:49:09 2017 Merged: Remove API check for interceptors to notify us of modifying the receiver Revision: ed43c63e26758e01631331ea820ef90baf4eb92f BUG= chromium:679345 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=hablich@chromium.org Review-Url: https://codereview.chromium.org/2634553002 . Cr-Commit-Position: refs/branch-heads/5.6@{#77} Cr-Branched-From: bdd3886218dfe76e8560eb8a18401942452ae859-refs/heads/5.6.326@{#1} Cr-Branched-From: 879f6599eee6e1dfcbe9a24bf688b261c03e9558-refs/heads/master@{#41014} [modify] https://crrev.com/6c7373d444966fbd4e1e4aabad0be4dc66b9eee3/src/objects.cc
,
Jan 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45f356d8a3036166754170d36019bb25ab128328 commit 45f356d8a3036166754170d36019bb25ab128328 Author: Jochen Eisinger <jochen@chromium.org> Date: Fri Jan 13 14:24:34 2017 Don't try to correctly notify V8 from the plugin interceptor Instead, I modified V8 to just cope with this behavior BUG= chromium:679345 TBR=haraken@chromium.org Review-Url: https://codereview.chromium.org/2625643002 Cr-Commit-Position: refs/heads/master@{#443212} (cherry picked from commit 8a4fe2fc28bb45f58445d5802057efdafbe7719b) Review-Url: https://codereview.chromium.org/2629083005 . Cr-Commit-Position: refs/branch-heads/2924@{#758} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/45f356d8a3036166754170d36019bb25ab128328/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-embedded-expected.txt [add] https://crrev.com/45f356d8a3036166754170d36019bb25ab128328/third_party/WebKit/LayoutTests/plugins/simple-expando.html [modify] https://crrev.com/45f356d8a3036166754170d36019bb25ab128328/third_party/WebKit/Source/bindings/core/v8/custom/V8HTMLPlugInElementCustom.cpp
,
Jan 17 2017
Verified on Mac OS 10.12.2 ,ubuntu 14.04 and windows 7 using chrome dev M57 #57.0.2984.0 and issue is fixed. Alert 5 is displayed on executing the given js code(comment #0) in web console. Attached screenshot for reference. Adding TE-Verified Labels. Thanks! |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by tkent@chromium.org
, Jan 9 2017Labels: Needs-Bisect