629859 - Drop support for localStorage[int]

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect)

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Why should this block 2.0?

Comment 2

[Honza Bambas (:mayhemer)]

This should go out ASAP, if the patch is simple.  But if there are voices again, then probably just release after Fx4.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

No, we should not do this for FF4. The risk of breaking existing sites is too big (there's always gecko-specific code paths).

Would be great to do asap after FF4 though.

Comment 4

[Hixie (not reading bugmail)]

My apologies again for this, by the way.

Comment 6

[Maciej Małecki]

Attachment: Patch to drop support for localStorage[int]

This patch duplicates behavior of nsStorage2SH::SetProperty, which "casts" each key to string.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 540332 [details] [diff] [review]
Patch to drop support for localStorage[int]

Don't we also need to remove the nsIDOMStorage::Key function implementation?

Comment 8

[Maciej Małecki]

Attachment: Removes nsIDOMStorage::Key implementation (in nsDOMStorage2) and definition

This patch removes definition of nsIDOMStorage::key (dom/interfaces/storage/nsIDOMStorage.idl) and it's implementation in nsDOMStorage2::Key (dom/src/storage/nsDOMStorage.cpp).

Comment 9

[Maciej Małecki]

Attachment: Remove relevant tests

I'm not sure if should we test to verify if this method is not working anymore or is just removing these tests enough?

Comment 10

[Honza Bambas (:mayhemer)]

Comment on attachment 546355 [details] [diff] [review]
Remove relevant tests

There are more test you have to remove this API checks from:
dom\tests\mochitest\localstorage\frameOrder.html
dom\tests\mochitest\localstorage\test_localStorageBase.html
dom\tests\mochitest\localstorage\test_localStorageBasePrivateBrowsing.html
dom\tests\mochitest\localstorage\test_localStorageBaseSessionOnly.html
dom\tests\mochitest\localstorage\test_localStorageKeyOrder.html
dom\tests\mochitest\sessionstorage\test_sessionStorageBase.html

Comment 11

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 546353 [details] [diff] [review]
Removes nsIDOMStorage::Key implementation (in nsDOMStorage2) and definition

Wait! the key function is still in the spec? Appears my previous suggestion to remove it was wrong. I didn't realize that it was a publicly exposed function required by spec.

Comment 12

[Scoobidiver (away)]

Did the approved patch land?