Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rethink client engine detection regarding reliability and need #8994

Open
level420 opened this issue May 6, 2016 · 16 comments
Open

rethink client engine detection regarding reliability and need #8994

level420 opened this issue May 6, 2016 · 16 comments
Labels
enhancement New features, give me a PR!!!

Comments

@level420
Copy link
Member

level420 commented May 6, 2016

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.

@level420 level420 changed the title Re-think client engine detection regarding reliability and need rethink client engine detection regarding reliability and need May 6, 2016
@oetiker
Copy link
Member

oetiker commented May 6, 2016

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
Copy link
Contributor

woprandi commented May 6, 2016

We could have the same debate about browser

@level420
Copy link
Member Author

level420 commented May 6, 2016

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
Copy link
Member

oetiker commented May 6, 2016

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
Copy link
Member Author

level420 commented May 6, 2016

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

@level420
Copy link
Member Author

level420 commented May 6, 2016

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
Copy link
Member

oetiker commented May 6, 2016

that sounds like a reasonable way

@oetiker
Copy link
Member

oetiker commented May 6, 2016

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
Copy link
Member Author

level420 commented Sep 16, 2016

From the discussion here and in PR #9180 (comment) , 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

  • webkit : e.g. Safari
  • blink : e.g. Chrome
  • gecko : e.g. Firefox
  • trident : Internet Explorer
  • edgehtml : MS Edge

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

@level420
Copy link
Member Author

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

@level420
Copy link
Member Author

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

@derrell
Copy link
Member

derrell commented Sep 16, 2016

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
Copy link
Member Author

level420 commented Sep 16, 2016

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

@level420
Copy link
Member Author

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

@derrell
Copy link
Member

derrell commented Sep 16, 2016

A new reality. Wow. Thanks.

@thron7
Copy link
Contributor

thron7 commented Sep 16, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, give me a PR!!!
Projects
None yet
Development

No branches or pull requests

5 participants