Bug 91668 - [V8] Improve Replaceable extended attribute
Summary: [V8] Improve Replaceable extended attribute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Erik Arvidsson
URL:
Keywords:
: 91148 (view as bug list)
Depends on:
Blocks: 91031
  Show dependency treegraph
 
Reported: 2012-07-18 14:11 PDT by Erik Arvidsson
Modified: 2012-12-06 09:25 PST (History)
9 users (show)

See Also:


Attachments
Patch (52.23 KB, patch)
2012-07-18 14:48 PDT, Erik Arvidsson
no flags Details | Formatted Diff | Diff
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 Details
Patch for landing (23.97 KB, patch)
2012-07-18 15:51 PDT, Erik Arvidsson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Erik Arvidsson 2012-07-18 14:11:59 PDT
[V8] Improve Replaceable extended attribute
Comment 1 Erik Arvidsson 2012-07-18 14:48:28 PDT
Created attachment 153094 [details]
Patch
Comment 2 Erik Arvidsson 2012-07-18 14:50:12 PDT
This issue was exposed by V8 fixing their bug with read only properties.
Comment 3 Adam Barth 2012-07-18 15:04:52 PDT
Comment on attachment 153094 [details]
Patch

Interesting.
Comment 4 WebKit Review Bot 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
Comment 5 WebKit Review Bot 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
Comment 6 Erik Arvidsson 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.
Comment 7 Erik Arvidsson 2012-07-18 15:51:59 PDT
Created attachment 153120 [details]
Patch for landing
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2012-07-18 16:54:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Erik Arvidsson 2012-07-19 11:56:59 PDT
*** Bug 91148 has been marked as a duplicate of this bug. ***
Comment 11 Arun Patole 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?
Comment 12 Erik Arvidsson 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?
Comment 13 Arun Patole 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'.
Comment 14 Erik Arvidsson 2012-12-06 07:47:06 PST
(In reply to comment #13)

delete is just broken. I filed bug 104263
Comment 15 Arun Patole 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!