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

Breaking fix - Edge browser.major should be Edge version, not EdgeHTML version #279

Closed
mikemaccana opened this issue Nov 22, 2017 · 4 comments
Closed

Comments

@mikemaccana
Copy link

@mikemaccana mikemaccana commented Nov 22, 2017

Currently parsedUserAgent.browser.major is incorrect for Edge - it reports parsedUserAgent.engine.major instead. I.E. Edge 41 incorrectly reports a parsedUserAgent.browser.major of 16 - this is the EdgeHTML version but not the browser version.

I'm currently working around this with something like:

var EDGEHTML_VS_EDGE_VERSIONS = {
	12: 0.1,
	13: 21,
	14: 31,
	15: 39,
	16: 41
}; 

var browserMajorVersion = parsedUserAgent.browser.major;
if ( browserName === 'Edge' ) {
	browserMajorVersion = EDGEHTML_VS_EDGE_VERSIONS[browserMajorVersion]
}

It would be cool if UA Parser JS did something similar. It's a breaking change though so should be in a major new version.

@mikemaccana mikemaccana changed the title Breaking fix - Edge should be edge version, not EdgeHTML version Breaking fix - Edge browser.major should be Edge version, not EdgeHTML version Nov 22, 2017
@mikemaccana mikemaccana changed the title Breaking fix - Edge browser.major should be Edge version, not EdgeHTML version Breaking fix - Edge browser.major should be Edge version, not EdgeHTML version Nov 22, 2017
@tb-mtg
Copy link

@tb-mtg tb-mtg commented Dec 6, 2018

I tried your demo site with Edge browser and it returned user agent

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36 Edge/18.17763

but incorrectly says browser version is 18.17763, but it actually is 44.17763.1.0 as show below from the app settings:

image

Can you fix so that is show the correct version?

@callaginn
Copy link

@callaginn callaginn commented Jan 31, 2019

@mikemaccana Nice; I was looking for something like this today. I've made a few tweaks:

  • Added two new versions of Edge. Hopefully, we won't have to deal with MS's crap much longer once they make the switch to Chromium.
  • Removed browserMajorVersion, in lieu of editing the parsed JSON result directly.
  • Added back minor point revision for browser.version property
  • Minor organization edits

Here's those adjustments with a user agent I pulled from one of my machines today:

var parser = new UAParser("mozilla/5.0 (windows nt 10.0; win64; x64) applewebkit/537.36 (khtml, like gecko) chrome/64.0.3282.140 safari/537.36 edge/17.17134");
var parsed = parser.getResult();

if (parsed.browser.name === 'Edge') {
    var ENGINE_TO_BROWSER = {
        12: 0.1,
        13: 21,
        14: 31,
        15: 39,
        16: 41,
        17: 42,
        18: 44
    };
    parsed.browser.major = ENGINE_TO_BROWSER[parsed.browser.major];
    parsed.browser.version = parsed.browser.major + '.' + parsed.browser.version.split('.')[1];
}

Result will be:

browser: {name: "Edge", version: "42.17134", major: 42}

@wessberg
Copy link

@wessberg wessberg commented Apr 10, 2021

It took me quite some time to track down the cause of a regression in production in which a Polyfill service I'm maintaining began returning invalid polyfills for Legacy edge because the browser versions have changed. I realize you did add a Breaking Change label to this, but since there was no new major SemVer version and no changelog is maintained, I didn't see it.

It is correct that the version range [12, 18] is technically the EdgeHTML versions, but that's what existing tooling like Caniuse, BrowserStack and MDN Web Docs use to refer to Legacy edge versions. It is also my clear impression that the vast majority of web developers refer to Legacy Edge by their EdgeHTML versions. Additionally, the EdgeHTML version is the only thing that is parseable from the User-Agent, which may be part of the explanation why that is more popular.

Here are a few examples:

BrowserStack:
Screen Shot 2021-04-10 at 3 42 46 PM

Caniuse:

Screen Shot 2021-04-10 at 3 44 50 PM

MDN:

Screen Shot 2021-04-10 at 3 46 13 PM

I actually can't think of a single tool or service that doesn't treat the EdgeHTML version as the browser version.

I would definitely suggest reverting this change to align with other tooling. I realize the original author had to work around this for their use case, but I for one now have to work around this behavior - and I would assume many others will too.

@faisalman
Copy link
Owner

@faisalman faisalman commented Apr 10, 2021

You're right, I originally made a fix for this issue in this branch: https://github.com/faisalman/ua-parser-js/tree/old-edge-fix but I forgot that this branch introduces a breaking change, so it should only be merged later when 0.8.x released, I'll revert this merge for now ..

@faisalman faisalman reopened this Apr 10, 2021
@faisalman faisalman closed this in 1d3c98a Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants