search

gshutler / useragent

HTTP User Agent parser
MIT License
179 stars 159 forks source link

Consider adding #real_version to Base #42

Open md5 opened 7 years ago

md5 commented 7 years ago

We are using Errbit for application error tracking and noticed that our error reports were erroneously saying "Internet Explorer 7" for some of our internal users who are actually using IE 11.

Errbit uses this gem for user agent identification and it turns out that our corporate settings for IE are flagging our site as being in the Intranet zone, which automatically turns on "compatibility mode" (see here for the gory details). Our actual site is using <meta http-equiv="x-ua-compatible" content="ie=edge"> to force the browser back out of compatibility mode, but by the time the server sees the User-Agent header it is too late for that to matter.

I would like to make a fix to Errbit to have it call #real_version to get the actual version of IE being used in compatibility mode, but I would rather not have to add a call to #respond_to?(:real_version) and bifurcate the code.

Would it make sense to add this to UserAgent::Browsers::Base:

def real_version
  version
end

If that were done, I could simply change the Errbit code to call #real_version instead of #version and avoid this issue with compatibility mode.

md5 commented 7 years ago

Also, based on the changes in https://github.com/gshutler/useragent/pull/30, I think that InternetExplorer#real_version should be calling #compact before calling #sort instead of the fix that was made in that PR.

gshutler commented 7 years ago

All sounds reasonable. I'd be happy to accept a pull request for real_version.

Could you provide a test/example of where #compact would work better?

md5 commented 7 years ago

@gshutler The user agent from the spec added in #30 or one of the ones in the description of the PR would be good examples. If you were to call real_version after parsing any of those strings, you would get ArgumentError, just as previously happened when calling compatibility_view?.

Now that I've had a day to think about this, however, I'm not sure it would be right for a User-Agent like "Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR * 1.1.4322; .NET CLR 2.0.50727; .NET CLR 3.0.04506.30)" (as described in #30) to return "7.0" as the #real_version.

It seems like you still don't want #real_version to raise ArgumentError in that case, but maybe it should just return nil since it isn't actually clear what the "real version" is. Of course, that would mean that my Errbit situation would still need special-case code, but at least if #real_version always existed and returned nil by default you could do something like #{user_agent.real_version || user_agent.version}.