podigee/device_detector

Tests not passing after updating regexes and fixtures

Closed this issue · 2 comments

Hello, I have just run the tasks for updating regexes and fixtures in order to fix some user agents not recognising the device type. This is now fixed, the device type is recognised as expected, however, the rests are failing.

I would like to get my hands on them and fix them, but I was wondering if that's the right way or you usually follow other approach.

Thanks.

Following with this issue, I have checked the history of changes in the original library and 3 new OS were added, adding them fixed 3 out of 4 failing tests.

The remaining one was fixed adding this:

# Chrome on Android passes the device type based on the keyword 'Mobile'
# If it is present the device should be a smartphone, otherwise it's a tablet
# See https://developer.chrome.com/multidevice/user-agent#chrome_for_android_user_agent
if t.nil? && os.family == 'Android' && ['Chrome', 'Chrome Mobile'].include?(name)
  if user_agent =~ build_regex('Chrome\/[\.0-9]* Mobile')
    t = 'smartphone'
  elsif user_agent =~ build_regex('Chrome\/[\.0-9]* (?!Mobile)')
    t = 'tablet'
  end
end

Following this chunk of code from the original library.

/**
 * Chrome on Android passes the device type based on the keyword 'Mobile'
 * If it is present the device should be a smartphone, otherwise it's a tablet
 * See https://developer.chrome.com/multidevice/user-agent#chrome_for_android_user_agent
 */
if (is_null($this->device) && $osFamily == 'Android' && in_array($this->getClient('name'), array('Chrome', 'Chrome Mobile'))) {
    if ($this->matchUserAgent('Chrome/[\.0-9]* Mobile')) {
        $this->device = DeviceParserAbstract::DEVICE_TYPE_SMARTPHONE;
    } else if ($this->matchUserAgent('Chrome/[\.0-9]* (?!Mobile)')) {
        $this->device = DeviceParserAbstract::DEVICE_TYPE_TABLET;
    }
}

Could this be merged if you think the update is good or let me know if I missed something or there is something that could have been done better?

Thanks.

Closed via #50