890405 - Remove nsIIDBVersionChangeEvent

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Jan Varga [:janv]]

Comment on attachment 771504 [details] [diff] [review]
Patch (v1)

Review of attachment 771504 [details] [diff] [review]:
-----------------------------------------------------------------

::: addon-sdk/source/lib/sdk/indexed-db.js
@@ -55,5 @@
>  exports.DOMException = Ci.nsIDOMDOMException;
>  exports.IDBCursor = Ci.nsIIDBCursor;
>  exports.IDBTransaction = Ci.nsIIDBTransaction;
>  exports.IDBOpenDBRequest = Ci.nsIIDBOpenDBRequest;
> -exports.IDBVersionChangeEvent = Ci.nsIIDBVersionChangeEvent;

IIRC you should inform someone from JetPack team about this change

::: dom/indexedDB/IDBEvents.cpp
@@ +19,1 @@
>  

Nit: I would move this above the usings with an empty line between them

::: dom/indexedDB/IDBEvents.h
@@ +40,5 @@
>  CreateGenericEvent(mozilla::dom::EventTarget* aOwner,
>                     const nsAString& aType,
>                     Bubbles aBubbles,
>                     Cancelable aCancelable);
>  

Nit: I would move this after |#define UPGRADENEEDED_EVT_STR "upgradeneeded"| with an empty line between them

Comment 3

[(no longer active)]

Comment on attachment 771504 [details] [diff] [review]
Patch (v1)

Review of attachment 771504 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/indexedDB/IDBEvents.cpp
@@ +19,1 @@
>  

OK, but that requires explicitly prefixing IDBVersionChangeEvent with its namespace prefix.

Comment 4

[(no longer active)]

Attachment: Patch (v2)

Over to Mossop for the add-on SDK change (a 1 line change, FWIW.)

Comment 5

[Jan Varga [:janv]]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #3)
> Comment on attachment 771504 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 771504 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/indexedDB/IDBEvents.cpp
> @@ +19,1 @@
> >  
> 
> OK, but that requires explicitly prefixing IDBVersionChangeEvent with its
> namespace prefix.

oh, then leave it as it is
but an extra empty line would be awesmome :)

Comment 6

[(no longer active)]

Attachment: Patch (v2)

Comment 7

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/121171dddc5b

Comment 8

[(no longer active)]

Backed out because of test failures

Comment 9

[(no longer active)]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #8)
> Backed out because of test failures

(https://hg.mozilla.org/integration/mozilla-inbound/rev/7ba9864d7a9e)

Comment 11

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/108049fedd49

Comment 12

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/0e9c10e9889aacc34cf77bd08f0e4fc426c5e9ed
Uplift Bug 890405 - Remove nsIIDBVersionChangeEvent to Add-on SDK. r=janv,mossop

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/108049fedd49

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e97532f5fd86

Comment 15

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Ed Morley [:edmorley UTC+1] from comment #14)
> https://hg.mozilla.org/mozilla-central/rev/e97532f5fd86

Empty patch

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to :Ms2ger from comment #15)
> (In reply to Ed Morley [:edmorley UTC+1] from comment #14)
> > https://hg.mozilla.org/mozilla-central/rev/e97532f5fd86
> 
> Empty patch

Yeah, apparently my original uplift of SDK code didn't blow away this bug's change like I thought it did. So an empty patch is the "correct", but unnecessary thing...