Bug 126200 – Handle (numeric) indexed access of DOMStringMap

• Status: RESOLVED
• Resolution: WORKSFORME
• Component: DOM

[Ryosuke Niwa]

Consider merging https://chromium.googlesource.com/chromium/blink/+/54785059007176b07b03810a3f98eb7afa4739b9

As the set of supported property names over DOMStringMap reflects the
attributes of an Element prefixed with 'data-', these naturally also
include numeric indexes.

To allow such index values to be used over DOMStringMap, extend its
IDL definition with indexed accessors.

Also address an infelicity in comparing attribute names to property
names if the property name uses a "-" but isn't followed by an ASCII
uppercase letter (e.g. "a-1".)

Extended test coverage to check for these variations also.

[Ahmad Saleem]

Not changed:

https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DatasetDOMStringMap.cpp#L87

IDL across all browsers:

https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/core/dom/dom_string_map.idl

https://hg.mozilla.org/mozilla-central/file/tip/dom/webidl/DOMStringMap.webidl

https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DOMStringMap.idl


___

DOMStringMap seems to point to DatasetDOMStringMap:

https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DOMStringMap.h

but I cannot find link of Chromium change to this:

https://github.com/WebKit/WebKit/blob/8afe31a018b11741abdf9b4d5bb973d7c1d9ff05/Source/WebCore/dom/DatasetDOMStringMap.h

___

I will leave this for others to comment whether this change is needed any more or not. Thanks!

[Ryosuke Niwa]

We handle numeric index getter just fine.

[Alexey Proskuryakov]

We do, however the Blink patch in question fixes more that that, and many subtests are still failing in WebKit.

Would you mind checking if your fix in bug 123890 covers all of these?

FAIL testGet('data-r-7', 'r-7') should be true. Was false.
FAIL testGet('data-r-7-k', 'r-7K') should be true. Was false.
...
FAIL testSet('-foo', 'dummy') should throw SyntaxError: Failed to set the '-foo' property on 'DOMStringMap': '-foo' is not a valid property name.. Threw exception SyntaxError: The string did not match the expected pattern..
...
FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string contains invalid characters.. Was false.
...
FAIL testDelete('data-r-2', 'r-2') should be true. Was false.
FAIL testDelete('data--r-2-', 'R-2-') should be true. Was false.
FAIL testDelete('data--r-2r', 'R-2r') should be true. Was false.
FAIL testDelete('data--r-2-----r', 'R-2----R') should be true. Was false.

[Ryosuke Niwa]

Yeah, I'm adding those test cases.

[Alexey Proskuryakov]

We are still failing this subtest after the fix:

FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string contains invalid characters.. Was false.

[Ryosuke Niwa]

(In reply to Alexey Proskuryakov from comment #5)
> We are still failing this subtest after the fix:
> 
> FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string
> contains invalid characters.. Was false.

That test case is bogus. No browser throws exception in that scenario.

[Alexey Proskuryakov]

I see the exception in Chrome 104.0.5112.101 (FAIL because their error message has changed since):

FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string contains invalid characters.. Threw exception InvalidCharacterError: Failed to set a named property on 'DOMStringMap': 'data-foo豈' is not a valid attribute name..

[Alexey Proskuryakov]

(and in Firefox too)

[Ryosuke Niwa]

(In reply to Alexey Proskuryakov from comment #7)
> I see the exception in Chrome 104.0.5112.101 (FAIL because their error
> message has changed since):
> 
> FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string
> contains invalid characters.. Threw exception InvalidCharacterError: Failed
> to set a named property on 'DOMStringMap': 'data-foo豈' is not a valid
> attribute name..

Huh, I guess I got confused while testing. Will address that in a separate bug.

[Ryosuke Niwa]

Tracking in https://bugs.webkit.org/show_bug.cgi?id=244174

[Ryosuke Niwa]

(In reply to Ryosuke Niwa from comment #9)
> (In reply to Alexey Proskuryakov from comment #7)
> > I see the exception in Chrome 104.0.5112.101 (FAIL because their error
> > message has changed since):
> > 
> > FAIL testSet('foo豈', 'dummy') should throw InvalidCharacterError: The string
> > contains invalid characters.. Threw exception InvalidCharacterError: Failed
> > to set a named property on 'DOMStringMap': 'data-foo豈' is not a valid
> > attribute name..
> 
> Huh, I guess I got confused while testing. Will address that in a separate
> bug.

Hm... actually, I don't get why other browsers are throwing in this case. U+F900 is clearly allowed in NameStartChar:
https://www.w3.org/TR/xml/#NT-Name

[Ryosuke Niwa]

This is Blink's implementation:
https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/core/dom/document.cc#L444

I think what happened here is that WebKit implements the latest specification:
https://commits.webkit.org/246206@main