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

Windows 8 version_compare #5794

Closed
etienne-martin opened this Issue Sep 3, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@etienne-martin
Contributor

etienne-martin commented Sep 3, 2018

I think there's an issue with the following if statement found in /DeviceDetector.php:

The comment says that any device with the Touch fragment running Windows 8 or later should be detected as a tablet:

/**
 * According to http://msdn.microsoft.com/en-us/library/ie/hh920767(v=vs.85).aspx
 * Internet Explorer 10 introduces the "Touch" UA string token. If this token is present at the end of the
 * UA string, the computer has touch capability, and is running Windows 8 (or later).
 * This UA string will be transmitted on a touch-enabled system running Windows 8 (RT)
 *
 * As most touch enabled devices are tablets and only a smaller part are desktops/notebooks we assume that
 * all Windows 8 touch devices are tablets.
 */

if (is_null($this->device) && ($osShortName == 'WRT' || ($osShortName == 'WIN' && version_compare($osVersion, '8.0'))) && $this->isTouchEnabled()) {
    $this->device = DeviceParserAbstract::DEVICE_TYPE_TABLET;
}

Executing version_compare('8.0', '8.0') returns 0, which is falsy. So in the end the if statement will only be true when the $osVersion is 8.1 or greater.

Am I missing something?

@sgiehl

This comment has been minimized.

Member

sgiehl commented Sep 6, 2018

guess you are right. that should maybe more likely be a version_compare() >= 0

etienne-martin added a commit to etienne-martin/matomo-device-detector that referenced this issue Sep 6, 2018

etienne-martin added a commit to etienne-martin/matomo-device-detector that referenced this issue Sep 6, 2018

sgiehl added a commit that referenced this issue Sep 9, 2018

@sgiehl sgiehl closed this Sep 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment