RESOLVED FIXED 91668
[V8] Improve Replaceable extended attribute
https://bugs.webkit.org/show_bug.cgi?id=91668
Summary [V8] Improve Replaceable extended attribute
Erik Arvidsson
Reported 2012-07-18 14:11:59 PDT
[V8] Improve Replaceable extended attribute
Attachments
Patch (52.23 KB, patch)
2012-07-18 14:48 PDT, Erik Arvidsson
no flags
Archive of layout-test-results from gce-cr-linux-08 (322.89 KB, application/zip)
2012-07-18 15:28 PDT, WebKit Review Bot
no flags
Patch for landing (23.97 KB, patch)
2012-07-18 15:51 PDT, Erik Arvidsson
no flags
Erik Arvidsson
Comment 1 2012-07-18 14:48:28 PDT
Erik Arvidsson
Comment 2 2012-07-18 14:50:12 PDT
This issue was exposed by V8 fixing their bug with read only properties.
Adam Barth
Comment 3 2012-07-18 15:04:52 PDT
Comment on attachment 153094 [details] Patch Interesting.
WebKit Review Bot
Comment 4 2012-07-18 15:28:43 PDT
Comment on attachment 153094 [details] Patch Attachment 153094 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13274629 New failing tests: fast/dom/Window/get-set-properties.html fast/dom/Window/window-property-shadowing.html
WebKit Review Bot
Comment 5 2012-07-18 15:28:55 PDT
Created attachment 153104 [details] Archive of layout-test-results from gce-cr-linux-08 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-08 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Erik Arvidsson
Comment 6 2012-07-18 15:50:57 PDT
I reverted the removal of the platform specific expectations. I had another change that touched readonly and I got this file on the wrong branch.
Erik Arvidsson
Comment 7 2012-07-18 15:51:59 PDT
Created attachment 153120 [details] Patch for landing
WebKit Review Bot
Comment 8 2012-07-18 16:54:37 PDT
Comment on attachment 153120 [details] Patch for landing Clearing flags on attachment: 153120 Committed r123047: <http://trac.webkit.org/changeset/123047>
WebKit Review Bot
Comment 9 2012-07-18 16:54:42 PDT
All reviewed patches have been landed. Closing bug.
Erik Arvidsson
Comment 10 2012-07-19 11:56:59 PDT
*** Bug 91148 has been marked as a duplicate of this bug. ***
Arun Patole
Comment 11 2012-12-04 22:56:23 PST
One question, if people are still following this bug. Why even non replaceable attributes are having replaceable setters with this change and both treated same way? e.g toolbar is replaceable but NodeList is not but both still have replaceable setters in V8 bindings V8DOMWindow.cpp. Just wanted to confirm if it is something that we intended or is it a bug?
Erik Arvidsson
Comment 12 2012-12-05 06:53:10 PST
(In reply to comment #11) > Just wanted to confirm if it is something that we intended or is it a bug? That is intended. What bug are you seeing?
Arun Patole
Comment 13 2012-12-05 21:29:48 PST
(In reply to comment #12) > (In reply to comment #11) > > Just wanted to confirm if it is something that we intended or is it a bug? > > That is intended. What bug are you seeing? Replaceable attribute looks to be behaving same as non-replaceable. from the IDL spec what I understand is, when we overwrite replaceable attribute and then again delete replaced variable, it should fall back to original attribute. And in case of non-replaceable, it shouldn't fall back to original after delete. Example: toolbar = document.getElementById('tmp').style.position alert (toolrbar); delete toolbar; alert (toolrbar); last alert shows, 'object BarInfo' which is correct as toolbar is replaceable. And for: NodeList = document.getElementById('tmp').style.position alert (NodeList); delete NodeList; alert (NodeList); last alert should show 'undefined' as NodeList is not replaceable attribute but it shows valid value like 'nodelist constructor'.
Erik Arvidsson
Comment 14 2012-12-06 07:47:06 PST
(In reply to comment #13) delete is just broken. I filed bug 104263
Arun Patole
Comment 15 2012-12-06 09:25:31 PST
(In reply to comment #14) > (In reply to comment #13) > > delete is just broken. I filed bug 104263 Ok, Thanks!
Note You need to log in before you can comment on or make changes to this bug.