Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the HTTP-BrowserDetect CPAN distribution.

Report information
The Basics
Id: 48727
Status: resolved
Priority: 0/
Queue: HTTP-BrowserDetect
People
Owner: Nobody in particular
Requestors: ROBINS [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: Normal
Broken in: 0.99
Fixed in: (no value)

History Show all quoted text
  Fri Aug 14 14:47:34 2009 ROBINS [...] cpan.org - Ticket created
Subject: Safari version is not correctly detected
With the following UserAgent string (Safari 4.0.2 for Windows) Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/530.19.2 (KHTML, like Gecko) Version/4.0.2 Safari/530.19.1 it reports version as 5.3, which is obviously wrong (it should be 4.0). I also want to point out that the phrase "KHTML, like Gecko" should be excluded as a match for the "gecko" method, as KHTML is a browser engine of its own, it is not Gecko (even though it likes to lie about itself). In fact, I'd suggest to add a khtml or webkit method aswell. KHTML could cover Konqueror (the KDE browser), and everything else that uses it (Safari+Chrome), and webkit method could just match Safari and Chrome.
  Mon Oct 19 15:54:25 2009 olaf [...] wundersolutions.com - Correspondence added
On Fri Aug 14 14:47:34 2009, ROBINS wrote: Show quoted text
> With the following UserAgent string (Safari 4.0.2 for Windows) > > Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/530.19.2 > (KHTML, like Gecko) Version/4.0.2 Safari/530.19.1 > > it reports version as 5.3, which is obviously wrong (it should be 4.0). > > I also want to point out that the phrase "KHTML, like Gecko" should be > excluded as a match for the "gecko" method, as KHTML is a browser engine > of its own, it is not Gecko (even though it likes to lie about itself). > In fact, I'd suggest to add a khtml or webkit method aswell. KHTML could > cover Konqueror (the KDE browser), and everything else that uses it > (Safari+Chrome), and webkit method could just match Safari and Chrome.
Hi, Good catch! Would you be willing to provide a patch for this? I can apply it and upload a new version ASAP. Best, Olaf
  Mon Oct 19 15:54:25 2009 The RT System itself - Status changed from 'new' to 'open'
  Tue Oct 20 04:18:07 2009 ROBINS [...] cpan.org - Correspondence added
Working on a patch now. Just be aware that actually making "KHTML, like Gecko" not match ->gecko() will cause several tests to actually fail I guess I should rewrite those tests to be accurate? That is -> not match gecko. This will probably bite someone.
  Tue Oct 20 05:17:20 2009 ROBINS [...] cpan.org - Correspondence added
Patch available in http://github.com/robinsmidsrod/http-browserdetect/. Pull request sent.
  Wed Oct 21 09:19:25 2009 olaf [...] wundersolutions.com - Correspondence added
On Tue Oct 20 05:17:20 2009, ROBINS wrote: Show quoted text
> Patch available in http://github.com/robinsmidsrod/http-browserdetect/. > Pull request sent.
Thanks very much for your patch. It has been applied and is now in version 1.02, which has been uploaded to the CPAN. Best, Olaf
  Wed Oct 21 09:19:26 2009 olaf [...] wundersolutions.com - Status changed from 'open' to 'resolved'
  Wed Oct 21 10:40:06 2009 peter [...] axomic.com - Correspondence added
Subject: Re: [rt.cpan.org #48727] Safari version is not correctly detected
Date: Wed, 21 Oct 2009 15:39:39 +0100
To: bug-HTTP-BrowserDetect [...] rt.cpan.org
From: Peter Walsham <peter [...] axomic.com>
Hi Olaf, Thanks for the many recent changes. Great to see some activity on this module. I just wanted to bring a couple of things to your attention regarding version numbers for Safari, and how they are produced by HTTP::BrowserDetect. Some people using HTTP::BrowserDetect for detecting Safari until now will have been used to it providing a version taken from "Safari/XXX". While I can see some sense in moving towards using "Version/XXX", this value won't always be available and therefore HTTP::BrowserDetect will start to generate 2 distinct types of version numbers for Safari. It will be important to keep some level of back compatibility for the many people out there already using the module (this includes me!). Here is a user agent string from Mac OS X 10.4.9. Please note it doesn't contain the version the user sees (i.e. "Version/XXX"). Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/418.8 (KHTML, like Gecko) Safari/419.3 In this example of Mac OS X on Windows, the user agent string does include "Version/XXX"... Mozilla/5.0 (Windows; U; Windows NT 5.1; en) AppleWebKit/522.12.1 (KHTML, like Gecko) Version/3.0.1 Safari/522.12.2 HTTP::BrowserDetect 0.99 produced version numbers of the form: Mac OS X Safari 5.25 Mac OS X Safari 5.3 Mac OS X Safari 5.31 Windows Vista Safari 5.25 Windows Vista Safari 5.3 As far as I understand it, with the proposed changes HTTP::BrowserDetect may now produce version numbers of the form: Mac OS X Safari 5.25 Mac OS X Safari 5.3 Mac OS X Safari 5.31 Windows Vista Safari 3.0.3 Windows Vista Safari 3.0.4 With 2 types of version number, it will be harder for people to test against Safari versions and it may break Safari compatibility with some applications. Cheers Pete Olaf Alders via RT wrote: Show quoted text
> Queue: HTTP-BrowserDetect > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=48727 > > > On Tue Oct 20 05:17:20 2009, ROBINS wrote:
>> Patch available in http://github.com/robinsmidsrod/http-browserdetect/. >> Pull request sent.
> > Thanks very much for your patch. It has been applied and is now in > version 1.02, which has been uploaded to the CPAN. > > Best, > > Olaf
-- Axomic Ltd 56 Compton Street London EC1V 0ET, UK t: +44 (0)870 850 0435 m: +44 (0)7812 122418 http://www.axomic.com The information contained in this message is confidential and should not be disclosed to any third party whether or not you are the intended addressee indicated in the message. Any views or opinions presented are solely those of the author and do not necessarily represent those of Axomic Ltd. If you are not the intended recipient, be advised that you have received this email in error and that any use, dissemination, printing, forwarding or copying of this e mail is strictly prohibited.
  Wed Oct 21 10:40:08 2009 The RT System itself - Status changed from 'resolved' to 'open'
  Wed Oct 21 11:17:36 2009 olaf [...] wundersolutions.com - Correspondence added
From: olaf [...] wundersolutions.com
On Wed Oct 21 10:40:06 2009, peterwalsham wrote: Show quoted text
> With 2 types of version number, it will be harder for people to test > against Safari versions and it may break Safari compatibility with some > applications. > > Cheers Pete
Hi Pete, What you're saying makes sense to me. Do you have any suggestion as to how we might go about making use of Version/XXX while still providing some backwards compatibility? Best, Olaf
  Wed Oct 21 12:41:35 2009 ROBINS [...] cpan.org - Correspondence added
What is apparent to me is that the number after Safari/XXX.XX.X is the WebKit version number. Maybe the module should be extended to also include the version number of the rendering engine used? This way it would be obvious that the Safari Mac strings are without browser version info and the version tag could just return 0 (or possibly -1 or undef), which means that the version of the rendering engine would have to be used instead to get a reliable number. I would suggest to also add a webkit() method, which would be true for Safari, Chrome and (possibly) Konqueror. Unfortunately this would break backwards compatibility, but it would be more sensible in the long run as more and more browsers use the same rendering engines. I would suggest that someone file a bug with Apple and ask them to include Version/X.X.X in future versions of Safari for Mac or change their user-agent string all-together to include the "public" version number after "Safari/", like most other browsers do. After all, they do have the WebKit version number in the UA string already.
  Thu Oct 22 05:49:37 2009 peter [...] axomic.com - Correspondence added
Subject: Re: [rt.cpan.org #48727] Safari version is not correctly detected
Date: Thu, 22 Oct 2009 10:49:21 +0100
To: bug-HTTP-BrowserDetect [...] rt.cpan.org
From: Peter Walsham <peter [...] axomic.com>
I don't think it is sensible to start changing the format given out by version() for browsers such as Safari or Opera (See also http://rt.cpan.org/Public/Bug/Display.html?id=50717) The following approach isn't very elegant but would not cause headaches for people already using HTTP::BrowserDetect Introduce a new method called versionPublic() that gives out the public version if it is available. e.g. Safari Windows version() "5.25" Safari Windows versionPublic() "3.0.4" Safari Mac version() "5.25" Safari Mac versionPublic() "" Opera version() "9.8" Opera versionPublic() "10.0" IE version() "8" IE versionPublic() "8" You might also want to introduce the methods majorPublic() and minorPublic() Developers could then test versionPublic() and fallback to using version() if the versionPublic() method is unavailable (e.g. HTTP::BrowserDetect<=1.02) or gives out an empty value (e.g. Safari Mac). Current users could choose to do nothing and their applications would carry on working ok regardless of which version of HTTP::BrowserDetect is installed. -- Axomic Ltd 56 Compton Street London EC1V 0ET, UK t: +44 (0)870 850 0435 m: +44 (0)7812 122418 http://www.axomic.com
  Thu Oct 22 23:04:45 2009 olaf [...] wundersolutions.com - Correspondence added
From: olaf [...] wundersolutions.com
On Thu Oct 22 05:49:37 2009, peterwalsham wrote: Show quoted text
> > I don't think it is sensible to start changing the format given out by > version() for browsers such as Safari or Opera (See also > http://rt.cpan.org/Public/Bug/Display.html?id=50717) > > The following approach isn't very elegant but would not cause headaches > for people already using HTTP::BrowserDetect > > Introduce a new method called versionPublic() that gives out the public > version if it is available. > > e.g. > > Safari Windows version() "5.25" > Safari Windows versionPublic() "3.0.4" > > Safari Mac version() "5.25" > Safari Mac versionPublic() "" > > Opera version() "9.8" > Opera versionPublic() "10.0" > > IE version() "8" > IE versionPublic() "8" > > You might also want to introduce the methods majorPublic() and
minorPublic() Show quoted text
> > Developers could then test versionPublic() and fallback to using > version() if the versionPublic() method is unavailable (e.g. > HTTP::BrowserDetect<=1.02) or gives out an empty value (e.g. Safari Mac). > > Current users could choose to do nothing and their applications would > carry on working ok regardless of which version of HTTP::BrowserDetect > is installed. > >
So, in the interest of "bugwards" compatibility, I can see that this makes sense. To sum up, the solution would be to revert version() to the previous (incorrect) behaviour and to introduce the following methods: public_version() public_minor() public_major() The repercussions would be that existing code would not have to be changed and that developers should probably be encouraged to use the public* methods for future code. Having said all of this, I think we can keep in mind the discussion in http://rt.cpan.org/Public/Bug/Display.html?id=50717 It probably would make sense to have some similar methods which return version/major/minor for the rendering engine, if this info is available, as that may be just as important. Any more thoughts on this? Does anyone feel motivated to provide a patch? I can really go either way on this, but if adding the new methods can be done elegantly, then that's probably a good solution for everybody.
  Thu Oct 22 23:22:49 2009 leedeleteeverythingbutthefirstthreecharacters [...] semel.net - Correspondence added
Subject: Re: [rt.cpan.org #48727] Safari version is not correctly detected
Date: Thu, 22 Oct 2009 23:22:31 -0400
To: bug-HTTP-BrowserDetect [...] rt.cpan.org
From: Lee Semel <lee [...] semel.net>
I'd think people would care about the rendering engine, in case they are modifying the their HTML to suit different rendering engines, or using the module to compile stats that help them decide how they code their sites - version alone don't provide that info. Lee On Thu, Oct 22, 2009 at 11:04 PM, Olaf Alders via RT < bug-HTTP-BrowserDetect@rt.cpan.org> wrote: Show quoted text
> Queue: HTTP-BrowserDetect > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=48727 > > > On Thu Oct 22 05:49:37 2009, peterwalsham wrote:
> > > > I don't think it is sensible to start changing the format given out by > > version() for browsers such as Safari or Opera (See also > > http://rt.cpan.org/Public/Bug/Display.html?id=50717) > > > > The following approach isn't very elegant but would not cause headaches > > for people already using HTTP::BrowserDetect > > > > Introduce a new method called versionPublic() that gives out the public > > version if it is available. > > > > e.g. > > > > Safari Windows version() "5.25" > > Safari Windows versionPublic() "3.0.4" > > > > Safari Mac version() "5.25" > > Safari Mac versionPublic() "" > > > > Opera version() "9.8" > > Opera versionPublic() "10.0" > > > > IE version() "8" > > IE versionPublic() "8" > > > > You might also want to introduce the methods majorPublic() and
> minorPublic()
> > > > Developers could then test versionPublic() and fallback to using > > version() if the versionPublic() method is unavailable (e.g. > > HTTP::BrowserDetect<=1.02) or gives out an empty value (e.g. Safari Mac). > > > > Current users could choose to do nothing and their applications would > > carry on working ok regardless of which version of HTTP::BrowserDetect > > is installed. > > > >
> > So, in the interest of "bugwards" compatibility, I can see that this > makes sense. To sum up, the solution would be to revert version() to > the previous (incorrect) behaviour and to introduce the following methods: > > public_version() > public_minor() > public_major() > > The repercussions would be that existing code would not have to be > changed and that developers should probably be encouraged to use the > public* methods for future code. > > Having said all of this, I think we can keep in mind the discussion in > http://rt.cpan.org/Public/Bug/Display.html?id=50717 > > It probably would make sense to have some similar methods which return > version/major/minor for the rendering engine, if this info is available, > as that may be just as important. > > Any more thoughts on this? Does anyone feel motivated to provide a > patch? I can really go either way on this, but if adding the new > methods can be done elegantly, then that's probably a good solution for > everybody. >
  Fri Oct 23 02:19:56 2009 ROBINS [...] cpan.org - Correspondence added
The suggestion about adding new types of version information I second. I am for reverting version/major/minor() for safari to its previous implementation. This way we would be left with the following methods: version() major() minor() public_version() public_major() public_minor() engine_version() engine_major() engine_minor() If the information is unavailable, I guess it's natural to return 0.
  Fri Oct 30 09:52:09 2009 peter [...] axomic.com - Correspondence added
Subject: Re: [rt.cpan.org #48727] Safari version is not correctly detected
Date: Fri, 30 Oct 2009 13:51:52 +0000
To: bug-HTTP-BrowserDetect [...] rt.cpan.org
From: Peter Walsham <peter [...] axomic.com>
Hi, I think the suggested approach is a good one. i.e. having methods of the form: version() public_version() engine_version() Best Wishes Pete -- Axomic Ltd 56 Compton Street London EC1V 0ET, UK t: +44 (0)870 850 0435 m: +44 (0)7812 122418 http://www.axomic.com
  Mon Nov 23 23:21:25 2009 olaf [...] wundersolutions.com - Correspondence added
From: olaf [...] wundersolutions.com
On Fri Oct 30 09:52:09 2009, peterwalsham wrote: Show quoted text
> > Hi, > > I think the suggested approach is a good one. i.e. having methods of the > form: > > version() > public_version() > engine_version() > > Best Wishes > > Pete >
Hey Guys, Does anyone feel motivated to provide a patch for this? Best, Olaf
  Mon Feb 08 11:04:24 2010 olaf [...] wundersolutions.com - Correspondence added
From: olaf [...] wundersolutions.com
On Mon Nov 23 23:21:25 2009, OALDERS wrote: Show quoted text
> Hey Guys, > > Does anyone feel motivated to provide a patch for this? > > Best, > > Olaf > >
The new methods have now been added and the tests are passing. I'm going to release this in the next couple of days if there are no objections. You can test it out using the source in the GitHub repository. Olaf
  Wed Feb 24 22:35:32 2010 olaf [...] wundersolutions.com - Status changed from 'open' to 'resolved'