Chrome allows elements with names to override collection properties

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Dave Moore]

If you have an HTMLSelectElement and it has an option that happens to be named 'length' you will get this element when accessing the length property, instead of the actual length. This also causes these accesses to be slower than they need to be.

[Dave Moore]

Created attachment 29233 [details]
Fix

This patch will only effect the chromium build, as it's in the bindings

[Dimitri Glazkov (Google)]

Comment on attachment 29233 [details]
Fix

Don't forget bug URL in ChangeLog description.

> +        (WebCore::collectionNamedPropertyGetter):
> +        (WebCore::nodeCollectionNamedPropertyGetter):

It's a good practice to add brief descriptions to each line on what's being done.

> +        if (!value.IsEmpty())
> +          return value;

4 spaces.

> +
> +        // Search local callback properties next to find IDL defined
> +        // properties.
> +        if (info.Holder()->HasRealNamedCallbackProperty(name))
> +          return v8::Handle<v8::Value>();

4 spaces.

Also, I think this should be return notHandledbyInterceptor().

> +        if (!value.IsEmpty())
> +          return value;

4 spaces.

> +        if (info.Holder()->HasRealNamedCallbackProperty(name))
> +          return v8::Handle<v8::Value>();

4 spaces, notHandledByInterceptor()

> [phone]  Dave Moore  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).

Need a good description of what the tests test.

> +            function test()
> +            {
> +                if (window.layoutTestController)
> +                    layoutTestController.dumpAsText();

Is this where we should put attempting to override length?

> +
> +                var sl = document.getElementById("sl");
> +                if (sl.length == 1)

[Dave Moore]

Created attachment 29236 [details]
Fix

[Dave Moore]

Created attachment 29237 [details]
Fix

Added a new tests to ensure that JS can override the length property on the HTMLSelectElement

[Dave Moore]

Created attachment 29240 [details]
Fix

Remove test for JS setting the length property.

[Dimitri Glazkov (Google)]

Comment on attachment 29240 [details]
Fix

looks good!

[Dimitri Glazkov (Google)]

Landed as http://trac.webkit.org/changeset/42247.