Last Comment Bug 532062 - localStorage/sessionStorage should return undefined (not null) for undefined keys
: localStorage/sessionStorage should return undefined (not null) for undefined ...
Status: RESOLVED FIXED
[mentor=jdm][lang=c++]
: dev-doc-needed, testcase
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal with 2 votes (vote)
: mozilla14
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
: Andrew Overholt [:overholt]
Mentors:
: 742468 (view as bug list)
Depends on: 740771
Blocks: 740357
  Show dependency treegraph
 
Reported: 2009-12-01 02:22 PST by bugzilla33
Modified: 2012-04-04 15:55 PDT (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
source (391 bytes, text/html)
2009-12-01 02:23 PST, bugzilla33
no flags Details
Patch v1 (15.10 KB, patch)
2012-03-30 05:06 PDT, :Ms2ger (⌚ UTC+1/+2)
honzab.moz: review+
Details | Diff | Splinter Review

Description bugzilla33 2009-12-01 02:22:00 PST
User-Agent:       Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; InfoPath.2; FDM)
Build Identifier: DOM Storage bug


empty values are casted to null instead undefined


Reproducible: Always

Steps to Reproduce:

Run attachment
Actual Results:  

Firefox 3.6: null


Expected Results:  

Firefox 3.6: undefined



Safari: undefined
Chrome 4.0: undefined
IE: undefined
Comment 1 bugzilla33 2009-12-01 02:23:50 PST
Created attachment 415369 [details]
source
Comment 2 bugzilla33 2009-12-01 02:26:00 PST
alert(localStorage.dfgergdfghsadgsdfasd) // shoul be undefined, no null
Comment 3 Olli Pettay [:smaug] 2009-12-02 03:15:36 PST
This depends on WebIDL specification, which is far from being ready, IIRC.
Comment 4 bugzilla33 2009-12-02 05:40:48 PST
In JS all no existing arrays/objects keys returns undefined.
NULL value, which had to be set by the programmer.
Comment 5 bugzilla33 2009-12-02 05:41:30 PST
Other browsers behave correctly.
Comment 6 bugzilla33 2009-12-02 05:43:17 PST
Try this code:

a=[]
alert(a.dfgergdfghsadgsdfasd)

Even Firefox returns undefined.
Comment 7 Olli Pettay [:smaug] 2009-12-02 05:57:30 PST
Ah, sorry, this is about localStorage.
Comment 8 Olli Pettay [:smaug] 2009-12-02 06:01:59 PST
I'm not quite sure what the summary should be.
Comment 9 bugzilla33 2009-12-02 06:51:29 PST
I think that the object localStorage should behave like any other objects and arrays. Should return undefined for undefined keys.

I am surprised other behavior for this one object in Firefox.

Other browsers do not have the specific behavior of the Storage object. They traditionally return undefined for non-existing keys.
Comment 10 bugzilla33 2009-12-02 06:53:30 PST
Where this idea that the Storage object behaved no-standard compared to all JS objects?
Comment 11 bugzilla33 2009-12-02 06:55:10 PST
In FF:

alert({}['qwerty']) // undefined
alert(localStorage['qwerty']) // null

mishmash
Comment 12 bugzilla33 2009-12-02 06:57:22 PST
Chrome 4 dev, IE, Safari:

alert({}['qwerty']) // undefined
alert(localStorage['qwerty']) // undefined

consistent behavior
Comment 13 Jonas Sicking (:sicking) No longer reading bugmail consistently 2009-12-02 07:15:28 PST
Other objects aside, localStorage is designed to behave like a JS object, so it's pretty clear that what we're doing is wrong here.
Comment 14 Olli Pettay [:smaug] 2009-12-02 07:33:13 PST
mayhemer knows a lot more about localStorage than I do.

I assume the current behavior is based on the following
http://dev.w3.org/html5/webstorage/
"The getItem(key) method must return a structured clone of the current value
associated with the given key. If the given key does not exist in the list
associated with the object then this method must return null."

So we probably map storage.foo to storage.getItem("foo") even if storage
doesn't have foo as a key. The *draft* spec doesn't seem to define this case
properly, but we should probably do what others do.

mayhemer, has this been discussed somewhere?
Comment 15 Honza Bambas (:mayhemer) 2009-12-02 07:38:48 PST
Olli, that's exactly what I wanted to write here. It's not specified what "dynamic" properties and array indexing should return. What we do is delegate to getItem every time the property is not a method of the storage. This leads to returning null when the key is not present in (was not stored to) dom storage.

It seems to be a problem in the spec, more then in Gecko itself. We should rise this in WHATWG or W3C forums.

Changing back to UNCONFIRMED at the moment.
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2009-12-02 09:04:28 PST
> Where this idea that the Storage object behaved no-standard compared to all JS
> objects?

That's an immediate consequence of it being a host object, not a native object.  I suggest reading the ES spec carefully.

Un-ccing from this spam-fest.
Comment 17 Jonas Sicking (:sicking) No longer reading bugmail consistently 2009-12-02 10:15:23 PST
I agree that the spec is unclear. However I always got the impression that Hixie intended for localStorage to largely behave like a normal JS object that got persisted.
Comment 18 bugzilla33 2009-12-23 05:16:15 PST
I think: FF has no caching for webStorage, look at speed tests:
https://bugzilla.mozilla.org/show_bug.cgi?id=536544
Comment 19 Anne (:annevk) 2009-12-23 13:07:03 PST
As I understand it from Web IDL and Web Storage localStorage.key should return undefined and localStorage.getItem("key") should return null if "key" is not actually a stored item.
Comment 20 Honza Bambas (:mayhemer) 2009-12-26 07:44:08 PST
-> me
Comment 21 Ernst de Haan 2010-04-10 01:34:04 PDT
I think one of the tests in Microsoft's HTML5 test suite may also point out this same error, see this test:
http://samples.msdn.microsoft.com/ietestcenter/HTML5/DOMStorage/sessionStorage_empty_key.htm

Expected would be a PASS, but in Firefox 3.6.3 (Mac OS X 10.6.3) I get a FAIL.

If that is a different issue, please let me know and I will create a separate bug report.

For the whole list of Microsoft's HTML5 tests, see:
http://samples.msdn.microsoft.com/ietestcenter/html5.htm
Comment 22 Martin Prebio 2010-05-26 09:20:16 PDT
Firefox 3.6.3:
console.log( typeof localStorage['inexistent'] );
object
console.log( localStorage['inexistent'] );
null

Chrome 5.0.375.55 beta:
console.log( typeof localStorage['inexistent']);
undefined
console.log( localStorage['inexistent']);
undefined

IE 8.0.7600.16385:
console.log( typeof localStorage['inexistent']);
undefined
console.log( localStorage['inexistent']);
undefined

Opera 10.53 Build 3374:
console.log( typeof localStorage['inexistent'] ); 
undefined
console.log( localStorage['inexistent']); 
undefined

;)
Comment 23 Aryeh Gregor (:ayg) (next working March 28-April 26) 2011-05-12 11:41:42 PDT
The spec requires that when you do direct property access, like localStorage.foo, nonexistent items are undefined.  When you do localStorage.getItem("foo"), they're null.  This is unreasonable, but it's what all other browsers do, and what the spec says.

Web Storage says:

"""
getter any getItem(in DOMString key);

...

The supported property names on a Storage object are the keys of each key/value pair currently present in the list associated with the object.
"""
http://dev.w3.org/html5/webstorage/#the-storage-interface

WebIDL says:

"""
If an interface supports named properties, then the interface definition must be accompanied by a description of what names the object can be indexed with at any given time. These names are called the supported property names.
"""
http://dev.w3.org/2006/webapi/WebIDL/#idl-named-properties

Then in the ECMAScript binding:

"""
Whenever a given string N becomes a supported property name on the host object the following steps must be followed:
...
10. Call the [[DefineOwnProperty]] method of O passing P, Property Descriptor { [[Get]]: get, [[Set]]: set, [[Enumerable]]: true, [[Configurable]]: configurable }, and false as arguments.
"""
http://dev.w3.org/2006/webapi/WebIDL/#named-properties

And in the same section:

"""
As soon as a given string N stops being supported property name, then its corresponding named property, if it exists, must be removed from the host object.
"""

So WebIDL requires that you behave as though you've added a property every time a property name becomes supported, and remove it when it becomes unsupported.  The value on getting that property is then defined to be as if you called the getter.  If the property name is not supported -- in our case, not a key in localStorage -- then it was never set to anything, so getting it is supposed to return undefined.

It would be nice if getItem() were consistent with direct access, but we have 3/4 implementations plus the spec against that, so Firefox should change to match everything else.
Comment 24 Honza Bambas (:mayhemer) 2012-01-18 08:31:23 PST
No time right now to work on this.
Comment 25 Josh Matthews [:jdm] (on vacation until Dec 5) 2012-02-07 20:44:10 PST
It sounds like we just need to add an else clause to the relevant blocks in nsStorage2SH::NewResolve and nsStorageSH::NewResolve that sets *objp to JSVAL_VOID. It worth trying to see what happens, at least.
Comment 26 :Ms2ger (⌚ UTC+1/+2) 2012-03-30 05:04:38 PDT
Got a patch. There's one issue though; we can't currently distinguish between null-as-a-real-key and null-because-there's-no-such-key. However, the latter isn't possible per spec, so we should fix that anyway. I filed bug 740771.
Comment 27 :Ms2ger (⌚ UTC+1/+2) 2012-03-30 05:06:08 PDT
Created attachment 610857 [details] [diff] [review]
Patch v1
Comment 28 :Ms2ger (⌚ UTC+1/+2) 2012-04-03 02:00:55 PDT
https://hg.mozilla.org/mozilla-central/rev/c410b2d6d570
Comment 29 Nickolay_Ponomarev 2012-04-04 15:55:29 PDT
*** Bug 742468 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.