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

RESOLVED INVALID

Status

()

RESOLVED INVALID
2 years ago
9 months ago

People

(Reporter: bevis, Assigned: bevis)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tw-dom] btpp-fixlater)

Attachments

(1 attachment, 1 obsolete attachment)

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
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8755712 [details] [diff] [review]
(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
Attachment #8755712 - Flags: review?(khuey)
(Assignee)

Updated

2 years ago
Summary: Fix the failure of "idbfactory_open10.htm" in IDB web-platform test → Set "allow_uncaught_exception" to true if window.onerror is expected for web-platform-test
(Assignee)

Comment 2

2 years ago
Created attachment 8755807 [details] [diff] [review]
(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
Attachment #8755712 - Attachment is obsolete: true
Attachment #8755712 - Flags: review?(khuey)
Attachment #8755807 - Flags: review?(khuey)
"In some implementations" sounds problematic. Either the spec requires an error event here, or it doesn't.
Whiteboard: btpp-active
(Assignee)

Comment 4

2 years ago
(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.
Flags: needinfo?(jonas)
(Assignee)

Comment 5

2 years ago
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.
Attachment #8755807 - Flags: review?(khuey)
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.
Flags: needinfo?(jonas)
(Assignee)

Comment 7

2 years ago
(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?
(Assignee)

Comment 8

2 years ago
(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.
(Assignee)

Comment 9

2 years ago
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.
Whiteboard: btpp-active → [tw-dom] btpp-fixlater
(Assignee)

Comment 10

9 months ago
This is pref-off in bug 1389913 since it's not available yet in spec v2.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.