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.96k stars 1.24k forks source link

sap.ui.Device doesn't recognize the new Chromium based Edge #2836

Closed mschleeweiss closed 4 years ago

mschleeweiss commented 4 years ago

OpenUI5 version: Any

Browser/version (+device/version): Edge 80.0.361.62

Steps to reproduce the problem:

  1. Install new Edge
  2. Open any page with UI5 running
  3. Call sap.ui.Device.browser.name

What is the expected result?

Name is "ed".

What happens instead?

Name is "cr".

Any other information? (attach screenshot if possible)

The user agent doesn't contain the word edge but only edg.

image

codeworrior commented 4 years ago

The current plan is to treat the Chromium based Edge like Chrome itself. We are not aware yet of any need to distinguish the two (at least not in the terms of Device.browser checks).

If you have a concrete requirement for differentiating between Chromium Edge and Chrome, please let us know.

mschleeweiss commented 4 years ago

We have an internal analytics tool to track which browser is used to access our Fiori Launchpad.

At the end of the day it doesn't really matter if its Chrome or Edge, it's just nice to know.

codeworrior commented 4 years ago

I see, sounds reasonable.

From our point of view, having more fine-grained Device.browser.names than boolean Device.browser.xyz properties, would create another source of confusion.

Maybe having an additional "long name" would be an option (navigator.userAgent would be the ultimate long name and it is already available to you :-) ).

@stopcoder what do you think?

mschleeweiss commented 4 years ago

I think it doesn't need to be more fine-grained.

The combination of browser name + browser version should be everything you need.

But right now the behaviour is as follows:

Browser Device.browser.name Device.browser.version
Edge 80.0.361.62 cr 80
Edge 44.18362 ed 18.18362
Chrome 80.0.3987.122 cr 80

In my opinion Edge 80 should return ed + 80, and it should be clear (to anyone using that information) that ed with a version number higher than 79 is the Chromium based Edge, and a lower number is the classic Edge

codeworrior commented 4 years ago

That's exactly what we DO NOT WANT.

It would require all code branches that currently check for Device.browser.edge to check for a version < 80 in addition. And all code branches that check for Device.browser.chrome or Device.browser.blink to also include Device.browser.edge with version >= 80.

And all CSS rules written for ed would need a version check, too and all CSS rules for cr would have to be extended to ed with an additional version check.

Treating Edge 80ff. like Chrome is much simpler from our point of view.

[Update] I found roughly 300 occurrences of such checks in JS code and 200 in CSS selectors.(in the whole SAPUI5 scope)

boghyon commented 4 years ago

I agree with @codeworrior. Aside from analytical point of view, it would cause great headaches to differentiate all Chromium browsers for technical purporses. IMHO sap.ui.Device.browser should stay informing browser engine (rather than browser's brand name) to combat technical inconsistencies.

Analytics tools shouldn't rely on sap.ui.Device.browser.name.

flovogt commented 4 years ago

Hi @mschleeweiss, first of all thanks a lot for bringing up this topic. MS Edge (Chromium) is a major shift especially for enterprises. We, the UI5 team, discussed this topic over a longer period of time und decided to not adjust the Device API. The job of the sap.ui.Device API is to report engines rather than brands and thus this behaviour is to stay. As long as there is no technical need for apps (and hopefully, there will be never such reasons), we also do not want to introduce new APIs allowing to tell Chrome and MS Edge (Chromium) apart. Apps and use cases requiring such a differentiation can make use of the browser’s user agent string. We have added this information to the documentation https://ui5.sap.com/#/topic/74b59efa0eef48988d3b716bd0ecc933. Also, we've adjusted the API documentation sap.ui.Device.browser, see here https://ui5.sap.com/#/api/sap.ui.Device.browser. With that I'll close this issue. Thanks a lot for your time to open this issue and providing clarity for the community. All the best, Florian