Bug 127358 - Improve the bindings of NodeList's name accessor
Summary: Improve the bindings of NodeList's name accessor
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: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-21 11:35 PST by Benjamin Poulain
Modified: 2014-01-26 02:26 PST (History)
6 users (show)

See Also:


Attachments
Patch (3.82 KB, patch)
2014-01-21 11:43 PST, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch (6.26 KB, patch)
2014-01-23 17:20 PST, Benjamin Poulain
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2014-01-21 11:35:15 PST
Improve the bindings of NodeList's name accessor
Comment 1 Benjamin Poulain 2014-01-21 11:43:12 PST
Created attachment 221768 [details]
Patch
Comment 2 Sam Weinig 2014-01-21 22:01:03 PST
Comment on attachment 221768 [details]
Patch

Is this something we can extend to all the nameGetters?
Comment 3 Benjamin Poulain 2014-01-22 20:23:36 PST
(In reply to comment #2)
> (From update of attachment 221768 [details])
> Is this something we can extend to all the nameGetters?

It seems indeed that several other classes suffer from the same issue. If someone can confirm this patch is correct, I'll change other objects.

It may be a good idea to nuke the pair canGetItemsForName()-nameGetter() entirely. Any opinion?
Comment 4 Geoffrey Garen 2014-01-23 15:03:38 PST
Comment on attachment 221768 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=221768&action=review

r=me

> Source/WebCore/bindings/js/JSNodeListCustom.cpp:57
> +        slot.setValue(this, ReadOnly | DontDelete | DontEnum, toJS(exec, globalObject(), item));

I think this works.

Please add a test to double-check that you haven't changed behavior of this property according to for-in enumeration and/or Object.getOwnPropertyDescriptor. Those are our best ways to test where the attributes set on the property are correct.
Comment 5 Benjamin Poulain 2014-01-23 17:20:43 PST
Created attachment 222049 [details]
Patch
Comment 6 Geoffrey Garen 2014-01-23 19:15:17 PST
Comment on attachment 222049 [details]
Patch

r=me
Comment 7 Benjamin Poulain 2014-01-26 02:26:53 PST
Committed r162801: <http://trac.webkit.org/changeset/162801>