SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.95k stars 1.23k forks source link

No check if offsetParent is null in oElement.offsetParent.nodeName.toUpperCase #3737

Closed lion9 closed 1 year ago

lion9 commented 1 year ago

OpenUI5 version:

current

Browser/version (+device/version): Chrome

Any other tested browsers/devices(OK/FAIL):

URL (minimal example if possible):

User/password (if required and possible - do not post any confidential information here):

Steps to reproduce the problem: Scroll to a ListItem inside a ScrollContainer

What is the expected result?

The ScrollContainer moves to the element

What happens instead?

Error is thrown because the framework tries to get a property of null

// do nothing if _$Container is not a (grand)parent of oElement
if (!this._$Container[0].contains(oElement) ||
    oElement.style.display === "none" ||
    **oElement.offsetParent**.nodeName.toUpperCase() === "HTML") {
        return this;
}
Screenshot 2023-04-14 at 19 34 03
boghyon commented 1 year ago

Scroll to a ListItem inside a ScrollContainer

Could you please share a minimal sample to reproduce the issue? You can edit https://jsbin.com/neyagus/edit?html,js,output, clone the edit, and share the new generated URL here.

Have you tried using the sap.m.ListBase#scrollToIndex API instead? Or is it not applicable in your case?

lion9 commented 1 year ago

scrollToIndex calls the same scrollToElement under the hood, with the same error being thrown

            setTimeout(function() {
                oScrollDelegate.scrollToElement(oItem.getDomRef(), null, [0, this._getStickyAreaHeight() * -1]);
                return resolve();
            }.bind(this), 0);
boghyon commented 1 year ago

As per https://github.com/SAP/openui5/blob/master/CONTRIBUTING.md#requirements-for-a-bug-report, a URL to a sample is needed in which the issue can be reproduced. Here are some templates you could extend accordingly:

lion9 commented 1 year ago

there was extra sap.m.App wrapper around an XML view, thus breaking the logic of child elements placement into the ScrollContainer