localStorage/sessionStorage should return undefined (not null) for undefined keys

RESOLVED FIXED in mozilla14

Status

()

Core
DOM
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: bugzilla33, Assigned: Ms2ger)

Tracking

({dev-doc-needed, testcase})

Trunk
mozilla14
dev-doc-needed, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=jdm][lang=c++])

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
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
(Reporter)

Updated

8 years ago
Component: General → DOM
Product: Firefox → Core
(Reporter)

Updated

8 years ago
Version: unspecified → 1.9.2 Branch
(Reporter)

Comment 1

8 years ago
Created attachment 415369 [details]
source
(Reporter)

Comment 2

8 years ago
alert(localStorage.dfgergdfghsadgsdfasd) // shoul be undefined, no null
(Reporter)

Updated

8 years ago
OS: Windows 7 → All
Hardware: x86 → All
QA Contact: general → general

Comment 3

8 years ago
This depends on WebIDL specification, which is far from being ready, IIRC.
Severity: major → normal
(Reporter)

Comment 4

8 years ago
In JS all no existing arrays/objects keys returns undefined.
NULL value, which had to be set by the programmer.
(Reporter)

Comment 5

8 years ago
Other browsers behave correctly.
(Reporter)

Comment 6

8 years ago
Try this code:

a=[]
alert(a.dfgergdfghsadgsdfasd)

Even Firefox returns undefined.

Comment 7

8 years ago
Ah, sorry, this is about localStorage.

Comment 8

8 years ago
I'm not quite sure what the summary should be.
(Reporter)

Comment 9

8 years ago
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.
(Reporter)

Comment 10

8 years ago
Where this idea that the Storage object behaved no-standard compared to all JS objects?
(Reporter)

Comment 11

8 years ago
In FF:

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

mishmash
(Reporter)

Comment 12

8 years ago
Chrome 4 dev, IE, Safari:

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

consistent behavior
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: empty values are casted to null instead undefined → localStorage should return undefined for undefined keys
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?
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.
Status: NEW → UNCONFIRMED
Ever confirmed: false
> 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.
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.
(Reporter)

Comment 18

8 years ago
I think: FF has no caching for webStorage, look at speed tests:
https://bugzilla.mozilla.org/show_bug.cgi?id=536544

Comment 19

8 years ago
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.
-> me
Assignee: nobody → honzab.moz
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Updated

7 years ago
Summary: localStorage should return undefined for undefined keys → localStorage/sessionStorage should return undefined (not null) for undefined keys

Updated

7 years ago
Keywords: testcase
Version: 1.9.2 Branch → Trunk

Comment 21

7 years ago
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

7 years ago
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

;)
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.
No time right now to work on this.
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
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.
Whiteboard: [mentor=jdm][lang=c++]
(Assignee)

Updated

5 years ago
Blocks: 740357
(Assignee)

Comment 26

5 years ago
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.
Assignee: nobody → Ms2ger
Depends on: 740771
(Assignee)

Comment 27

5 years ago
Created attachment 610857 [details] [diff] [review]
Patch v1
Attachment #610857 - Flags: review?(honzab.moz)
Attachment #610857 - Flags: review?(honzab.moz) → review+
(Assignee)

Comment 28

5 years ago
https://hg.mozilla.org/mozilla-central/rev/c410b2d6d570
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
(Assignee)

Updated

5 years ago
Keywords: dev-doc-needed

Updated

5 years ago
Duplicate of this bug: 742468
You need to log in before you can comment on or make changes to this bug.