Spec clarification regarding implied steps

[stelar7]

From working on the Ladybird implementation of this spec, I've encountered some parts that could use clarification..

• Opening a database connection step 10.8 says If the upgrade transaction was aborted, but there is no mention elsewhere in the steps where the upgrade transaction comes from. (Also not how to check if its aborted.. See next bullet)

• The only way to tell if a transaction was aborted seems to be if the transaction has an error, but when calling IDBTransaction::abort(), the error is set to null (It calls abort a transaction with a null parameter).
  So this abort is not marking the transaction as aborted?

• In abort a transaction, step 6.3.1, it says to use the open request associated with transaction, but there is no spec steps that sets this anywhere

• In delete a database, step 5. wants the set of all connections associated with db, but there are no steps to create this association

• In IDBFactory::open, step 4 creates a new open request, and then dispatches an event to this during step 5.2.3. Then later it is returned as a IDBOpenDBRequest. Is the open request a IDBRequest? And if so, is there a reason to not create an IDBOpenRequest directly in step 4?

[inexorabletash]

Thanks! This sort of feedback is great. This spec predates the modern style of writing out explicit algorithms, so it's unsurprising that there are still gaps to plug.

• Opening a database connection step 10.8 says If the upgrade transaction was aborted, but there is no mention elsewhere in the steps where the upgrade transaction comes from.

Agreed. It looks like an easy fix would be for the "upgrade a database" steps to return the transaction.

• The only way to tell if a transaction was aborted seems to be if the transaction has an error, but when calling IDBTransaction::abort(), the error is set to null (It calls abort a transaction with a null parameter).
  So this abort is not marking the transaction as aborted?

The intent is that an explicit abort() does abort the transaction, but the value reported via the error getter is null.

So yeah, that means that the error state of a transaction is used incorrectly. A minimal fix probably looks like:

• Defining the error state as initially unset, but set to a value (which can include null) if the transaction is aborted.
• Revising the "abort a transaction" steps to say "If error is not set, set transaction’s error to error."

• In abort a transaction, step 6.3.1, it says to use the open request associated with transaction, but there is no spec steps that sets this anywhere

This is specified as part of "open request" - but could be made more explicit as part of "upgrade a database" steps. For example, just saying "request's transaction is transaction".

I'll note that the steps to "asynchronously execute a request" don't explicitly associate the transaction with the new request either.

• In delete a database, step 5. wants the set of all connections associated with db, but there are no steps to create this association

Yeah, it's just heavily implied in "database connection" and "opening a database connection". Making it explicit would be nice.

• In IDBFactory::open, step 4 creates a new open request, and then dispatches an event to this during step 5.2.3. Then later it is returned as a IDBOpenDBRequest. Is the open request a IDBRequest? And if so, is there a reason to not create an IDBOpenRequest directly in step 4?

This is a quirk of how and when the spec was written. The spec text mostly deals in abstract constructs and assumes the WebIDL-defined interfaces are the reflection of those in script. So it refers to a "connection" not an IDBDatabase, and a "request" not an IDBRequest. A browser implementer could equate the constructs to the C++ objects, which are distinct from the JS objects.

More recently written specs dispense with that level of abstraction. I'd welcome a future editor reworking this!

PRs welcome for improvements to any/all of the above.