1274938 - Set "allow_uncaught_exception" to true if window.onerror is expected for web-platform-test

Status: RESOLVED INVALID

(Core :: Storage: IndexedDB, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

Error could be fired by the pending IDBRequests if IDBTransaction is aborted according to the step 4 of |Steps for aborting a transaction| in [1].

We could set {allow_uncaught_exception: true} to increase the tolerance of this test case.

[1] http://w3c.github.io/IndexedDB/#steps-for-aborting-a-transaction

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v1) Set "allow_uncaught_exception" to true for this test case.

IMHO, window.onerror is expected if we don't set preventDefault to the error event of the IDBOpenRequest/IDBRequest at line#10, 22, 23, 43, 44 of [1] according to the step 4 of |Steps for aborting a transaction| in [2].

In addition, the purpose of this test case is to ensure that the change to the db will be reverted if the transaction is aborted.

Hence, I set "allow_uncaught_exception" to true to ignore these errors instead of having any change in gecko.

[1] http://w3c.github.io/IndexedDB/#steps-for-aborting-a-transaction
[2] https://hg.mozilla.org/mozilla-central/annotate/16663eb3dcfa759f25b5e27b101bc79270c156f2/testing/web-platform/tests/IndexedDB/idbfactory_open10.htm

Comment 2

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: (v2) Set "allow_uncaught_exception" to true if window.onerror is expected.

In our implementation, window.onerror will be triggered if the event target of the error is an IDBRequest.

For idbfactory_open10.htm,
window.onerror is expected if we don't set preventDefault to the error event of the IDBOpenRequest/IDBRequest at line#10, 22, 23, 43, 44 of [1] according to the step 4 of |Steps for aborting a transaction| in [2].

For idbtransaction_objectStoreNames.html,
tx.onabort in the test case of |IDBTransaction.objectStoreNames - value after abort| is required for the verification, so we cannot call |e.preventDefault()| in |tx.objectStore('s1').add(0, 0).onerror| [3] to suppress the window.error thrown in our implementation.

Hence, I'd like to set "allow_uncaught_exception" to true to ignore these errors instead of having any change in gecko.

Same approach has been done in idbobjectstore_createIndex6-event_order.htm and idbobjectstore_createIndex7-event_order.htm by [4], BTW.


[1] https://hg.mozilla.org/mozilla-central/annotate/16663eb3dcfa759f25b5e27b101bc79270c156f2/testing/web-platform/tests/IndexedDB/idbfactory_open10.htm
[2] http://w3c.github.io/IndexedDB/#steps-for-aborting-a-transaction
[3] https://hg.mozilla.org/mozilla-central/annotate/16663eb3dcfa759f25b5e27b101bc79270c156f2/testing/web-platform/tests/IndexedDB/idbtransaction_objectStoreNames.html#l123
[4] https://github.com/w3c/web-platform-tests/commit/2d54c040aae8dc601767c99142e0c68b49d96546

Comment 3

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

"In some implementations" sounds problematic. Either the spec requires an error event here, or it doesn't.

Comment 4

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to :Ms2ger from comment #3)
> "In some implementations" sounds problematic. Either the spec requires an
> error event here, or it doesn't.

m..., you are right. Thanks for rising this concern.

The window.error was not specified in IDB Spec, but was done intendedly in bug 607729 to "allow IndexedDB events to propagate, and error events to hit window.error".

ni? jonas for more information of bug 607729 and better suggestion of fixing these failure web-platform tests caused by window.onerror but expected in our implementation.

Comment 5

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8755807 [details] [diff] [review]
(v2) Set "allow_uncaught_exception" to true if window.onerror is expected.

cancel the review first due to the concern in comment 4.

Comment 6

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

This was debated when we wrote the spec, but I think by accident it didn't make it into the spec. I think we should propose to get it added to the V2 version of the spec which is currently being worked on.

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Jonas Sicking (:sicking) from comment #6)
> This was debated when we wrote the spec, but I think by accident it didn't
> make it into the spec. I think we should propose to get it added to the V2
> version of the spec which is currently being worked on.

Is there any history of this debate, then we can propose it again with proper reasons to v2 spec?

Comment 8

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng[:bevistseng][:btseng] from comment #7)
> (In reply to Jonas Sicking (:sicking) from comment #6)
> > This was debated when we wrote the spec, but I think by accident it didn't
> > make it into the spec. I think we should propose to get it added to the V2
> > version of the spec which is currently being worked on.
> 
> Is there any history of this debate, then we can propose it again with
> proper reasons to v2 spec?

It's still an open issue in https://github.com/w3c/IndexedDB/issues/49
I'll expect this to be addressed eventually in spec v2.

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

Expecting spec & web-platform test change according to spec author's comment in https://github.com/w3c/IndexedDB/issues/49

Change btpp status to fixlater.

Comment 10

[Bevis Tseng[:bevis][:btseng](Exited)]

This is pref-off in bug 1389913 since it's not available yet in spec v2.