qooxdoo / qooxdoo

qooxdoo - Universal JavaScript Framework
http://qooxdoo.org
Other
765 stars 260 forks source link

rethink client engine detection regarding reliability and need #8994

Open level420 opened 8 years ago

level420 commented 8 years ago

The framework tries to detect the client engines by using unique properties offered through window.navigator e.g. the user agent string or proprietary properties.

As the HTML spec says that window.navigator.product = "Gecko" is standard, this property is useless to distinguish the engines.

Currently, especially for the "real" gecko engine, the proprietary buildID attribute is used, which may go away as others proprietary attributes did in the past (window.navigator.mozApps in V47 and window.controllers in V30). There is an ongoing discussion regarding deprecation/hiding of buildID at https://bugzilla.mozilla.org/show_bug.cgi?id=583181

This issue is to open up the discussion on how we could change the detection of the rendering engines to be more reliable or to discuss if this is needed at all.

oetiker commented 8 years ago

Qooxdoo is providing a 'nice' interface, which is great just because the details of the implementation seem to be necessarily ugly as there is no standard approach ...

Hopefully the requirement for this interface will becomes less as time goes by and browsers become progressively more standard adherent ...

Looking throught the code, I see that for now qx.bom.client.Engine is used quite frequently https://github.com/qooxdoo/qooxdoo/search?utf8=%E2%9C%93&q=qx.bom.client.Engine

woprandi commented 8 years ago

We could have the same debate about browser

level420 commented 8 years ago

The negative impact failing to detect gecko is, that it reports a "default" version of 1.9.0.0, not the real engine version. But for the rendering engine name itself the fallback is to report it as gecko anyway.

The code also lacks of detecting the blink engine. It is currently detected as webkit. But implementing this may also lead to problems, as the specific code currently executed for qx.core.Environment.get("engine.name") === 'webkit' may then fail on current chrome.

oetiker commented 8 years ago

we can make sure that qooxdoo is consistent internally (grep for webkit) users who have browser specific code in their own codebase will have to grep -r too ... but I think that is ok ... especially since I find that I have rarely to write any browser specific code in my own qooxdoo apps as qooxdoo is shielding me very well from this kind of problems.

level420 commented 8 years ago

Here some discussion regarding blink detection on stackoverflow: http://stackoverflow.com/questions/20655470/how-to-detect-blink-in-chrome

level420 commented 8 years ago

Here ist what I've found at mozilla developer network: https://developer.mozilla.org/en-US/docs/Browser_detection_using_the_user_agent#Rendering_engine

oetiker commented 8 years ago

that sounds like a reasonable way

oetiker commented 8 years ago

after all we will always have to do some magic, if it werent like this, then we would not need such a facility in the first place and everyone could just read the appropriate bom property

level420 commented 7 years ago

From the discussion here and in PR https://github.com/qooxdoo/qooxdoo/pull/9180#issuecomment-247565393 , it might be a good process to leave the current implementation which delivers engine.name as is, as this avoids incompatibilities with the current framework code, contributions and apps out there.

Instead, as @oetiker suggested, we should have a new key engine.realname which delivers, well the real engine name of the current browser. These are currently

We maybe should search for a good engine detection library which is integrable in the framework ( like sizzle ).

level420 commented 7 years ago

Here is maybe a good candidate: https://github.com/ded/bowser

level420 commented 7 years ago

A fiddle with bowser : https://jsfiddle.net/hwqy4xz4/2/

derrell commented 7 years ago

I'm running Chrome Version 53.0.2785.92 beta (64-bit) That fiddle identified it as

{ "name": "Chrome", "chrome": true, "version": "53.0", "blink": true, "linux": true, "a": true }

Is that right? blink?

level420 commented 7 years ago

@derrell exactly! At ~ version 25 chrome changed from webkit to blink

level420 commented 7 years ago

It was version 28 where chrome changed from webkit to blink: https://en.m.wikipedia.org/wiki/Blink_(web_engine)

derrell commented 7 years ago

A new reality. Wow. Thanks.

thron7 commented 7 years ago

As for the environment key I'm very much a fan of namespaces. I'm also very much a fan of not naming the real thing "real", but what you want to name it in the first place. So let me suggest re-naming the old engine.name to something like engine.name.legacy, and keep engine.name for the new values. It would be easy to fix in the framework, and an easy automatic migration for custom code (['"]engine.name["']="engine.name.legacy"). Then everybody can consider where and how it makes sense in their code to revert back to the "real" key and work with the new values.

Of course, somebody using the new qx version but omits running the migration might face runtime issues. But we had semantic changes before (where an existing construct attains a new behavior), and that's what the automatic migration is for, after all.