Q's knobs turn no more

[humdinger]

This is hrev54249.

'Q' is an old MIDI app (​source) available from HaikuDepot.

I haven't used it for a very long time, but it did work probably a year ago. Now all its knobs can't be turned any more. It loads a saved track correctly and shows the properly set knobs, though.
Some Haiku change has broken BeOS compatibility, or Q does something naughty...

(Please adjust this ticket's category.)

[pulkomandy]

Another victim of hrev53446. I had the same problem in Sawteeth. Fixed in Q (​https://github.com/HaikuArchives/Q/commit/e9aac3dfba971de1fce26d127922c0a537f1b3cf), but I'd like to request again that we revert this change as it clearly breaks BeOS compatibility and I see no reason for doing it.

[pulkomandy]

Proposal for reverting the change that breaks compatibility with BeOS and fixes no actual problem as far as I can see: ​https://review.haiku-os.org/c/haiku/+/2851

[X512]

Base class MessageReceived must be called for unhandled messages as described in BeBook: ​https://www.haiku-os.org/legacy-docs/bebook/BHandler.html#BHandler_MessageReceived. Q code was broken.

[pulkomandy]

Yes, it was, but we still try to provide BeOS compatibility. It was confirmed that BeOS does not behave as documented here and now we know at least two apps rely on that behavior. We are lucky that we have the sources for these two but it may not always be the case.

We may try a more compromising solution, by having both the B_KEY_DOWN message and the KeyDown() call triggered. But it should not be possible for a BView subclass to intercept the message and prevent KeyDown to be called.

So:

• In BWindow, call both view->KeyDown(string, bytes - 1); and target->MessageReceived(message); when handling the messages
• In BView, remove the default handling of the messages.

I think this would allow to manage input events in MessageReceived if you really want to, but at least you couldn't accidentally break the other functions by forgetting to call the parent handler, and restore Behavior more similar to BeOS.

[X512]

It was confirmed that BeOS does not behave as documented here and now we know at least two apps rely on that behavior.

I think that documented behavior should be preferred and also current code allows more flexible message handling. 1 of 2 apps is already fixed and Sawteeth can be fixed in same way. It's better to keep good architecture instead keeping compatibility with broken applications that depend on undocumented behavior.

[pulkomandy]

Sawteeth is already fixed.

I don't think the architecture is broken here. It disagrees a bit with the documentation, but it's not completely stupid. And in any case, we try to provide BeOS compatibility. Maybe you don't care or priorize it differently, but I do.

[X512]

Why handle BView messages in BWindow? It make no sense and break common architecture practice.

There are already many BeOS incompatibilities like illegal heap usage (double free etc.) that work in BeOS, but crash in Haiku. The are no meaning and sometimes even not possible to replicate BeOS bugs and undocumented behavior. Haiku is not BeOS clone and intended to be modern system.

[korli]

Replying to pulkomandy:

Sawteeth is already fixed.

I don't think the architecture is broken here. It disagrees a bit with the documentation, but it's not completely stupid. And in any case, we try to provide BeOS compatibility. Maybe you don't care or priorize it differently, but I do.

Here we have an old application recompiled for Haiku, so we can't detect that it is expecting the BeOS behavior anyhow. Binary versus source compatibility.

[pulkomandy]

Yes, I'm not worried too much about the recompiled apps however, these can be fixed if we decide the new behavior is desirable. But this breaks BeOS apps, and in this case I don't think the change is worth it. I'm not saying we should allow the same double-frees as BeOS does, that would be crazy, but, we do say we have binary compatibility. And binary compatibility is not attained by looking at the docs, it is known that they are inaccurate or wrong in several places.

The question is just, where do we put the balance between compatibility with existing apps, and improving the behavior? In this case I think the improvement is smaller than the breakage (admittedly, the breakage is also small, for now, but there may be more of it to be discovered).

[waddlesplash]

Well, the change has been in place for months and little breakage has been so far discovered. So I say we should leave it in for now, and if there is more breakage in unfixable apps, then we can add an ABI flag.