Implement `propagateMessages` in `MessageMixin` to fix `ControlMessageProcessor`

LukasHeimann commented 2 years ago

As a developer, I want to use the ControlMessageProcessor to propagate messages to controls without databinding. In order for that to work, controls must implement a propagateMessages function.

However, a refactoring in 2016 removed that bit of functionality from UI5's standard controls, leaving message processing only working with a databinding: https://github.com/SAP/openui5/commit/344286249226e6877ef27fa6cb374908960dc242

Please fix this by adding an implementation for propagateMessages to MessageMixin, so messaging works as documented (and as before the regression 6 years ago). To me, it looks like you should be able to reuse nearly everything inside the already existing refreshDataState method, so this wouldn't even be a huge effort: https://github.com/SAP/openui5/blob/master/src/sap.ui.core/src/sap/ui/core/message/MessageMixin.js#L37-L79

Thank you for considering to fix this long-standing bug! I can also provide a small PR to fix this myself and get the behavior restored in a timely manner.

Kind regards Lukas

LukasHeimann commented 2 years ago

Sample code for snippix:

<!DOCTYPE html>
<html>
    <head>
        <meta http-equiv='X-UA-Compatible' content='IE=edge'>
        <meta charset="utf-8">

        <title>Mobile App with XML View with JSON Data</title>

        <script id='sap-ui-bootstrap'
            src='/sapui5/resources/sap-ui-core.js'
            data-sap-ui-theme='sap_fiori_3'
            data-sap-ui-libs='sap.m'
            data-sap-ui-compatVersion='edge'></script>

        <!-- define a new (simple) XML view
         - using data binding for the Button text
         - binding a controller method to the Button's "press" event
         note: typically this would be a standalone file -->

        <script id="myXml" type="text/xmldata">
            <mvc:View xmlns:mvc="sap.ui.core.mvc" xmlns="sap.m" controllerName="myController" displayBlock="true">
                <App>
                    <Page title="Hello">
                        <Button text="send message" press="doSomething"/>
                        <Input id="test"/>
                    </Page>
                </App>
            </mvc:View>
        </script>

        <script>
            sap.ui.controller("myController", {
                onInit: function() {
                    const oView = this.getView();
                    const oMessageManager = sap.ui.getCore().getMessageManager();
                    oView.setModel(oMessageManager.getMessageModel(), "message");
                    oMessageManager.registerObject(oView, true);
                },
                doSomething: function() {
                    sap.ui.getCore().getMessageManager().removeAllMessages();
                    sap.ui.getCore().getMessageManager().addMessages([
                      new sap.ui.core.message.Message({
                        message: "Message",
                        type: sap.ui.core.MessageType.Error,
                        additionalText: "Additional Text",
                        target: this.createId("test/value"),
                        processor: new sap.ui.core.message.ControlMessageProcessor(),
                      }),
                    ]);
                }
            });
            sap.ui.xmlview({ viewContent: jQuery('#myXml').html() }).placeAt("content");
        </script>

    </head>
    <body id='content' class='sapUiBody'>
    </body>
</html>

As you can see, only the default implementation logging a warning in ManagedObject is hit -- the Input control doesn't implement the method (but easily could via the Mixin, if it were fixed)

H4ze commented 1 year ago

Created an incident to re-enable the messaging without data binding again or to adjust the documentation: 2270121480

H4ze commented 1 year ago

Dear @LukasHeimann,

we checked your suggestion to add the propagateMessage to the MessageMixin but we decided to not submit the change.

The reason for this decision is, that this change could possibly lead to unwanted behavior in case within controller coding (or also within controls) the validation events are not used properly.

For both cases we found some coding where either within controls or controllers the ManagedObject.fireValidationError is called without a corresponding ManagedObject.fireValidationSuccess call. In the worst case, this could lead to messages which are created for a control but never removed again and also for setting the valueState to "Error" without resetting the valueState in case the issue is solved.

I created a jsbin to demonstrate the behavior but you need a setup including the propagateMessage change in order to reproduce it. propagteMessage Example

The risk of changing the behavior is from our point of view to high in comparison to the additional value of such an solution.

Kind regards, Johannes

SAP / openui5

Implement `propagateMessages` in `MessageMixin` to fix `ControlMessageProcessor` #3553