Bug 98814 – IndexedDB: Key paths should support non-ASCII identifiers

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Joshua Bell]

IDB spec http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-key-path says:

A key path is a DOMString or Array that defines how to extract a key from a value. A valid key path is one of:
* An empty DOMString.
* A DOMString containing a JavaScript identifier [ECMA-262].
* A DOMString containing multiple Javascript identifiers separated by periods (ASCII character code 46) [ECMA-262].
* A non-empty Array containing only DOMStrings conforming to the above requirements.

And ECMA-262 http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf#page=29 sayeth:

"Identifier Names are tokens that are interpreted according to the grammar given in the ―Identifiers‖ section of 
chapter 5 of the Unicode standard, with some small modifications. An Identifier is an IdentifierName that is not 
a  ReservedWord (see 7.6.1). The Unicode identifier grammar is based on both normative and informative 
character categories specified by the Unicode Standard. The characters in the specified categories in version 
3.0 of the Unicode standard must be treated as in those categories by all conforming ECMAScript 
implementations.

"This standard specifies specific character additions: The dollar sign ($) and the underscore (_) are permitted 
anywhere in an IdentifierName."

And this is followed by a grammar.

However, the WebKit IDB implementation uses:

static inline bool isSafeIdentifierStartCharacter(UChar c)
{
    return isASCIIAlpha(c) || (c == '_') || (c == '$');
}

static inline bool isSafeIdentifierCharacter(UChar c)
{
    return isASCIIAlphanumeric(c) || (c == '_') || (c == '$');
}

... so we fail http://w3c-test.org/webapps/IndexedDB/tests/submissions/Opera/keypath.htm

[Joshua Bell]

Created attachment 168072 [details]
Patch

[Joshua Bell]

dgrogan@, alecflett@ - can you take a look?

[Joshua Bell]

Created attachment 168097 [details]
Patch

[Joshua Bell]

Updated patch fixes escaping and adds invalid keypath tests (other Unicode categories as initial/trailing identifier characters)

[Joshua Bell]

+haraken@ who wrote implemented the current lexer

[Joshua Bell]

And note that I filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=19450 about an edge case (reserved words being excluded - FF doesn't and I don't think it should be required)

[Joshua Bell]

Also, I did not add additional cases to WebKit/chromium/tests/IDBKeyPathTest.cpp - it seemed redundant with the layout test once the basic API was exercised from C++.

[David Grogan]

Comment on attachment 168097 [details]
Patch

LGTM

After looking at 3.1.5 Key Path in the idb spec and taking a cursory glance at ecma-262, this seems reasonable.

[Joshua Bell]

haraken@ - r?

[Kentaro Hara]

Comment on attachment 168097 [details]
Patch

Looks OK

[Joshua Bell]

Created attachment 168734 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 168734 [details]
Patch for landing

Clearing flags on attachment: 168734

Committed r131356: <http://trac.webkit.org/changeset/131356>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.