Merge "Promise spec" fix from Firefox Topic is solved

Talk about code development, features, specific bugs, enhancements, patches, and similar things.
Forum rules
Please keep everything here strictly on-topic.
This board is meant for Pale Moon source code development related subjects only like code snippets, patches, specific bugs, git, the repositories, etc.

This is not for tech support! Please do not post tech support questions in the "Development" board!
Please make sure not to use this board for support questions. Please post issues with specific websites, extensions, etc. in the relevant boards for those topics.

Please keep things on-topic as this forum will be used for reference for Pale Moon development. Expect topics that aren't relevant as such to be moved or deleted.
llkjdsaB

Merge "Promise spec" fix from Firefox

Unread post by llkjdsaB » 2018-11-13, 16:21

When using a secure webmail client (tutanota.com), I often get errors (without useful error text: "Oops! Tell us what happened."). The tutanota dev team who apparently does get useful error messages with these reports replies to me,
tutanota dev team member wrote:The problem is that PaleMoon (as its origin, Firefox, did) does not implement Promise spec correctly and because of that the database transaction is closed too soon. Firefox fixed this around version 60 and Safari fixed that in 12.1. We need to apply workaround for PaleMoon as well.
I just want to say that in general both old browsers and low-supported browsers make our life much harder. Please vote for PaleMoon to merge that fix so that they can merge patch which implements Promise microtasks correctly.
It may be helpful for others as well. I'm using Palemoon 28.1.0, downloaded as a binary from palemoon.org.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Merge "Promise spec" fix from Firefox

Unread post by Moonchild » 2018-11-13, 16:39

Of course we aim to be spec complaint. It would help greatly to know what exactly the spec issue is, or, alternatively, a reference bugzilla bug number where Mozilla fixed it in Firefox.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
athenian200
Contributing developer
Contributing developer
Posts: 1498
Joined: 2018-10-28, 19:56
Location: Georgia

Re: Merge "Promise spec" fix from Firefox

Unread post by athenian200 » 2018-12-10, 14:55

I believe the OP must be referring to this. It's a Bugzilla post that talks about microtasks and Promise issues that were fixed in Firefox 60, just like the OP mentions. It also says a lot of websites are designed around this particular broken behavior in Firefox because it took them three years to fix it, so it may not be that critical to get in a rush if you're targeting compatibility with Firefox versions older than 60 anyway, but it's something to consider for the future at least.

https://bugzilla.mozilla.org/show_bug.c ... 193394#c14
"The Athenians, however, represent the unity of these opposites; in them, mind or spirit has emerged from the Theban subjectivity without losing itself in the Spartan objectivity of ethical life. With the Athenians, the rights of the State and of the individual found as perfect a union as was possible at all at the level of the Greek spirit." -- Hegel's philosophy of Mind

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Merge "Promise spec" fix from Firefox

Unread post by Moonchild » 2018-12-11, 00:44

I'm sorry but what is proposed there is illogical.
See: http://jsbin.com/kaqume/1/edit?js,console,output
They are looking at 3 different tasks here:
  1. A sync task (mutationobserver)
  2. An async task (promise)
  3. A delayed sync task (timeout)
Logically, this is the correct order for them to execute, which is exactly what happens in Pale Moon. There is one event (click) handled by 2 objects. First, in the time line, event handlers and sync event occur, then the async events, then the delayed sync events. This will result in, color coded by phase: click (o1), mutate (o1), click (o2), mutate (o2), promise (o1), promise (o2), timeout (o1), timeout (o2)
What the BZ bug wants to achieve is "click" "promise" "mutate" "click" "promise" "mutate" "timeout" "timeout".
Having the (asynchronous) promise fire before the (synchronous) mutate makes no sense, and I refuse to implement something that illogical. If this is in the spec this way then that is IMO a spec bug because it shouldn't happen that way.

In other words: If you want things to occur synchronously in sequential and defined tasks, then don't use promises. Their very nature is deferred processing.
Last edited by Moonchild on 2018-12-11, 00:50, edited 3 times in total.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite


User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Merge "Promise spec" fix from Firefox

Unread post by Moonchild » 2018-12-11, 01:40

Looking into the HTML spec we are actually encouraged to "experiment with different implementations" in this context:
Pausing is highly detrimental to the user experience, especially in scenarios where a single event loop is shared among multiple documents. User agents are encouraged to experiment with alternatives to pausing, such as spinning the event loop or even simply proceeding without any kind of suspended execution at all, insofar as it is possible to do so while preserving compatibility with existing content. This specification will happily change if a less-drastic alternative is discovered to be web-compatible.

In the interim, implementers should be aware that the variety of alternatives that user agents might experiment with can change subtle aspects of event loop behavior, including task and microtask timing. Implementations should continue experimenting even if doing so causes them to violate the exact semantics implied by the pause operation.
Source: https://html.spec.whatwg.org/#pause

The suggested merging of the "promise spec" from Firefox would cause pausing like this, because synchronous processing of object 1 has to be paused until the promise of it is resolved, and then again for the second object. In fact, it's worse, because the second object has to wait for the first object's promise to be handled, as well as its own promise, making all of this sequential while it should not be. "click" -PAUSE- "promise" "mutate" "click" -PAUSE- "promise" "mutate" "timeout" "timeout".

That's unacceptable.

In our current implementation, processing can occur in parallel as is the intention for these kinds of event-driven actions.
Last edited by Moonchild on 2018-12-11, 01:48, edited 4 times in total.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
athenian200
Contributing developer
Contributing developer
Posts: 1498
Joined: 2018-10-28, 19:56
Location: Georgia

Re: Merge "Promise spec" fix from Firefox

Unread post by athenian200 » 2018-12-12, 09:59

I definitely think your reasoning, based on the specs you've linked, makes more sense than the reasoning used in that thread or the solutions they proposed. It seems like the people developing Firefox aren't really sure what they're doing and are just trying to make things work the way people expect them to work (probably based on how Chrome does things).

What I noticed is that the web developer mentions "the database," and I've seen a lot of talk about IndexedDB requiring promises to be handled in a specific manner, and this being a highly atypical yet supported (for some reason?) edge case.

https://stackoverflow.com/questions/283 ... 5#28388805

This thread seems to suggest that Firefox has implemented microtasks improperly.
Last edited by athenian200 on 2018-12-12, 10:06, edited 2 times in total.
"The Athenians, however, represent the unity of these opposites; in them, mind or spirit has emerged from the Theban subjectivity without losing itself in the Spartan objectivity of ethical life. With the Athenians, the rights of the State and of the individual found as perfect a union as was possible at all at the level of the Greek spirit." -- Hegel's philosophy of Mind

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Merge "Promise spec" fix from Firefox

Unread post by Moonchild » 2018-12-12, 11:22

athenian200 wrote:It would seem so far to me that certain developers are probably misusing the Promise spec. [...] (probably based on how Chrome does things)
Promises have been pushed (very, too?) hard by the powers that be as "the way to do everything now". Many a dev has cut themselves on that by suddenly having to deal with dangers of asynchronicity (re-entrancy, data being removed from under functions, etc.). As such there's a lot of misuse and it seems that IndexedDB transactions were at one time analyzed and written using promises by one person using Chrome, and the quirks of that implementation became "the way to do things" which was short-sighted.
This is exactly the kind of thing, a very practical example, of the danger of becoming an implementation monoculture (now that Microsoft will also be moving to Blink in Edge) where one implementation becomes the determining method to do something.
athenian200 wrote:and I've seen a lot of talk about IndexedDB requiring promises to be handled in a specific manner, and this being a highly atypical yet supported (for some reason?) edge case.
It's only supported because Chrome does. :/

Database transactions don't require promises at all, but can be handled in a more performant way with them (because you can fire off a DB query in a promise, and then handle the result whenever the DB is done processing it). The issue here seems to be that the actual DB connections are not handled asynchronously, and as such when the promise is fired, the DB connection is lost while the promise (the query) has not been resolved yet. That is not a fault of promises themselves, but rather the caller making assumptions that the promise would resolve before processing continues (assuming a microtask is handled sequentially, while it is not), which is exactly the kind of pausing the spec says to avoid by changing the implementation, even if it makes timing/sequence changes.
Last edited by Moonchild on 2018-12-13, 07:49, edited 1 time in total.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
athenian200
Contributing developer
Contributing developer
Posts: 1498
Joined: 2018-10-28, 19:56
Location: Georgia

Re: Merge "Promise spec" fix from Firefox

Unread post by athenian200 » 2018-12-12, 14:02

I've worked with databases before (although I'm not a programmer), and even I think it's weird because I know database access usually isn't done using asynchronous functions. You generally want to follow a linear process... connect to the database, grab the data, and then close the connection. There's no room for asynchronous behavior in that, because if you do those steps out of order, it won't work. The thing that bugs me is that it seems like people are focusing more on improving speed by any means possible and using clever hacks than on doing things right. It's also weird to note how the lack of alternative implementations leads to even potentially well-intentioned Chrome developers wondering if it's a Firefox bug, and Firefox developers wondering if it's a Chrome bug, without a lot of other implementations to refer to besides one another.

Anyway, I found something that suggests the Chromium devs have admitted that their current behavior might be a spec violation that they created in an attempt to speed up performance, and that they're trying to get the spec changed so they don't have a "performance regression." Typical, huh? If they have so much power they can just casually juggle the idea of having the specs changed, then what does having a standard even mean?

https://bugs.chromium.org/p/v8/issues/detail?id=5694
Last edited by athenian200 on 2018-12-12, 14:10, edited 2 times in total.
"The Athenians, however, represent the unity of these opposites; in them, mind or spirit has emerged from the Theban subjectivity without losing itself in the Spartan objectivity of ethical life. With the Athenians, the rights of the State and of the individual found as perfect a union as was possible at all at the level of the Greek spirit." -- Hegel's philosophy of Mind

User avatar
Isengrim
Board Warrior
Board Warrior
Posts: 1325
Joined: 2015-09-08, 22:54
Location: 127.0.0.1
Contact:

Re: Merge "Promise spec" fix from Firefox

Unread post by Isengrim » 2018-12-12, 17:37

athenian200 wrote:If they have so much power they can just casually juggle the idea of having the specs changed, then what does having a standard even mean?
a.k.a. Ascrod
Linux Mint 19.3 Cinnamon (64-bit), Debian Bullseye (64-bit), Windows 7 (64-bit)
"As long as there is someone who will appreciate the work involved in the creation, the effort is time well spent." ~ Tetsuzou Kamadani, Cave Story

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35474
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Merge "Promise spec" fix from Firefox

Unread post by Moonchild » 2018-12-13, 19:31

@athenian200: you are absolutely correct in that database transactions require a progression of events, but the approach is a little different here.
If you're dealing with an event-driven application, which in practice any web application is, then doing things purely synchronously can be considerably slower than leveraging the nature of the environment you are in. You can use promises to tell the browser to fetch data from the database while you go on your merry way, and process the result "when it is ready and fetched". The problem with this particular implementation however is that the transaction is broken up as far as I can tell and that the DB connection and closure thereof is simply not connected to the data retrieval method. That's still fine if you make sure to check there are no running queries before you close the db connection (which you can check via promises as well, if you want -- i.e. "close the connection when data has been retrieved". (e.g. (open connection).then(retrieve data).finally(close connection))

No matter what the exact cause is, it is clearly a problem in the caller, i.e. the JS implementation, that needs to be fixed. Either make the DB connect and transaction a single function, or make sure to check the promise has been resolved before closing the transaction. Not this half-way one or the other thing.
Last edited by Moonchild on 2018-12-13, 19:34, edited 2 times in total.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

Locked