WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 85937
[Chromium][Forms] HTMLOptionsCollection doesn't have indexed properties on property enumeration
https://bugs.webkit.org/show_bug.cgi?id=85937
Summary
[Chromium][Forms] HTMLOptionsCollection doesn't have indexed properties on pr...
yosin
Reported
2012-05-08 17:57:06 PDT
ConsoleMessage._printArray takes elements of array object of inspector's internal representation from numeric property name. However, HTMLOptionsCollection object don't have such properties. Even though, getting a element works in scripts in original HTML and JavaScript console. Below is sample steps: 1. Visit data:text/html,<form><select id="s"><option value="1">one</option><option value="2">two</option></select></form> 2. Invoke JavaScript Console 3. Enter "keys(documents.forms)" ["0", "length"] 4. Enter "keys(s.options)" ["selectedIndex", "length"] 5. Enter: "s.options [] Note: Safari's JavaScript console works fine.
Attachments
Patch
(5.42 KB, patch)
2012-05-08 21:28 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Patch
(5.49 KB, patch)
2012-05-08 21:42 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cr-linux-03
(6.07 MB, application/zip)
2012-05-09 00:27 PDT
,
WebKit Review Bot
no flags
Details
Patch 3
(7.18 KB, patch)
2012-05-09 00:43 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Patch 4
(7.19 KB, patch)
2012-05-09 02:34 PDT
,
yosin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
yosin
Comment 1
2012-05-08 20:54:15 PDT
Object.keys(select.options) should return an array of property keys containing numeric index value, e.g. ["0", "1", "selectedIndex", "length"]. Here is test result of browsers with simple test
http://jsfiddle.net/MbtFP/5/
* FF12 OK * Chrome18 NOT -- no "0" and "1" * IE9 OK * Opera11 OK
yosin
Comment 2
2012-05-08 21:28:44 PDT
Created
attachment 140863
[details]
Patch
yosin
Comment 3
2012-05-08 21:34:14 PDT
This bug blocks
http://crbug.com/121890
- DOM Select Object always returns empty list for options attribute
Kentaro Hara
Comment 4
2012-05-08 21:34:55 PDT
Comment on
attachment 140863
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=140863&action=review
What is happening for other interfaces that have indexed properties? I mean, should this change be for HTMLOptionsCollection only?
> Source/WebCore/ChangeLog:10 > +
Please describe other browsers' behavior in ChangeLog, to justify your change.
yosin
Comment 5
2012-05-08 21:42:35 PDT
Created
attachment 140864
[details]
Patch
WebKit Review Bot
Comment 6
2012-05-09 00:27:24 PDT
Comment on
attachment 140864
[details]
Patch
Attachment 140864
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/12662002
New failing tests: inspector/console/console-format-collections.html
WebKit Review Bot
Comment 7
2012-05-09 00:27:46 PDT
Created
attachment 140872
[details]
Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
yosin
Comment 8
2012-05-09 00:43:53 PDT
Created
attachment 140875
[details]
Patch 3
Kentaro Hara
Comment 9
2012-05-09 00:45:54 PDT
(In reply to
comment #4
)
> What is happening for other interfaces that have indexed properties? I mean, should this change be for HTMLOptionsCollection only?
Would you clarify the above point? I am not sure if we should fix HTMLOptionsCollection specific code or more general IndexedProperty code.(In reply to
comment #0
)
> ConsoleMessage._printArray takes elements of array object of inspector's internal representation from numeric property name. However, HTMLOptionsCollection object don't have such properties. > > Even though, getting a element works in scripts in original HTML and JavaScript console. > > Below is sample steps: > > 1. Visit data:text/html,<form><select id="s"><option value="1">one</option><option value="2">two</option></select></form> > 2. Invoke JavaScript Console > 3. Enter "keys(documents.forms)" > ["0", "length"] > 4. Enter "keys(s.options)" > ["selectedIndex", "length"] > 5. Enter: "s.options > [] > > Note: Safari's JavaScript console works fine.
yosin
Comment 10
2012-05-09 02:14:00 PDT
(In reply to
comment #9
)
> (In reply to
comment #4
) > > What is happening for other interfaces that have indexed properties? I mean, should this change be for HTMLOptionsCollection only? > > Would you clarify the above point? I am not sure if we should fix HTMLOptionsCollection specific code or more general IndexedProperty code.(In reply to
comment #0
)
This change should affect HTMLOptionCollection only. Enumerator should be attached for indexed properties. In IDL which is specified by IndexedGetter(HTMLCollection, HTMLSelect), NumericIndexer({Float,Int,UInt}{16,32,64,8}Array), or CustomGetOwnPropertySlot (no use case, HTMLAppletElement, HTMLEmbedElement, HTMLObjectElement have JSCustomGetOwnPropertySlotAndDescriptor). Code generator doesn't generate code for NumericIndexer, CustomGetOwnPropertySlot is handlbed by (%indexerSpecialCases hash map, we should add this attribute to these interfaces and remove %indexerSpecialCases). For interfaces which have IndexedGetter, GenerateImplementationIndexer generates code of calling setCollectionXXX<T> which setting enumerator if interfaces inherited from Node or no CustomSetter, e.g. HTMLSelectElement interface has IndexedGetter and CustomSetter attribute, HTMLCollection has IndexedGetter attribute but no CustomSetter.
Kentaro Hara
Comment 11
2012-05-09 02:21:52 PDT
Comment on
attachment 140875
[details]
Patch 3 View in context:
https://bugs.webkit.org/attachment.cgi?id=140875&action=review
The patch looks good to me.
> LayoutTests/fast/forms/select/options-indexed-properties.html:11 > +</select>
Nit: Remove this.
> LayoutTests/fast/forms/select/options-indexed-properties.html:23 > +for (var propertyKey in options) { > + properties[propertyKey] = options[propertyKey]; > +} > +shouldBe('properties[0].value', '"a"'); > +shouldBe('properties[1].value', '"b"');
You might want to write this as follows: shouldBeEqualToString('options[0].value', "a"); shouldBeEqualToString('options[1].value', "b");
> LayoutTests/fast/forms/select/options-indexed-properties.html:29 > +
Nit: Remove the redundant new line.
yosin
Comment 12
2012-05-09 02:34:21 PDT
Created
attachment 140893
[details]
Patch 4
WebKit Review Bot
Comment 13
2012-05-09 04:10:10 PDT
Comment on
attachment 140893
[details]
Patch 4 Clearing flags on attachment: 140893 Committed
r116513
: <
http://trac.webkit.org/changeset/116513
>
WebKit Review Bot
Comment 14
2012-05-09 04:10:42 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug