silverstripe-archive / silverstripe-mobile

Mobile support module for SilverStripe CMS
http://silverstripe.org/mobile/
BSD 3-Clause "New" or "Revised" License
35 stars 36 forks source link

Detection is inefficient #40

Open selay opened 11 years ago

selay commented 11 years ago

I am using this module and there is an issue with detection. Bu I needed to optimize it for my own use because it each time when is_mobile is called, does the same checking to see if it is mobile or not. In reality, if something is not a mobile, it can not change itself during the user session to become a mobile. So, once the detection is done, it should be stored in a session to return the previous evaluation result if the session is set.
Doing so much string match, search, etc with headers to identify user agent etc, is not very efficient for php. So, can just keep in session once it is done. and this user will be marked as mobile during this session. I hope you consider this one in your next release or update.

selay commented 11 years ago

The modified function should be like this:

   public static function is_mobile($agent = null) {
  //return true;
    $isMobile = false;
   if (isset($_GET['flush']))
     Session::clear('isMobile');
    $isMobile=Session::get('isMobile'); //check session -Elmin
      if (!$isMobile&&$isMobile!==0){   //if first time, not sessioned
    if(!$agent) $agent = $_SERVER['HTTP_USER_AGENT'];
    $accept = isset($_SERVER['HTTP_ACCEPT']) ? $_SERVER['HTTP_ACCEPT'] : '';

    switch(true) {
        case(self::is_iphone()):
            $isMobile = true;
            break;
        case(self::is_android()):
            $isMobile = true;
            break;
        case(self::is_opera_mini()):
            $isMobile = true;
            break;
        case(self::is_blackberry()):
            $isMobile = true;
            break;
        case(self::is_palm()):
            $isMobile = true;
            break;
        case(self::is_win_phone()):
            $isMobile = true;
            break;
        case(self::is_windows()):
            $isMobile = true;
            break;
        case(preg_match('/(up.browser|up.link|mmp|symbian|smartphone|midp|wap|vodafone|o2|pocket|kindle|mobile|pda|psp|treo)/i', $agent)):
            $isMobile = true;
            break;
        case((strpos($accept, 'text/vnd.wap.wml') !== false) || (strpos($accept, 'application/vnd.wap.xhtml+xml') !== false)):
            $isMobile = true;
            break;
        case(isset($_SERVER['HTTP_X_WAP_PROFILE']) || isset($_SERVER['HTTP_PROFILE'])):
            $isMobile = true;
            break;
        case(in_array(strtolower(substr($agent, 0, 4)), self::mobile_index_list())):
            $isMobile = true;
            break;
    }
    if (!$isMobile) $isMobile=0;
      Session::set('isMobile', $isMobile);   
 } 
    if(!headers_sent()) {
        header('Cache-Control: no-transform');
        header('Vary: User-Agent, Accept');
    }

    return $isMobile;
}
wilr commented 11 years ago

@ielmin please submit your change via github so a) you get credit for it, b) we can easily see what you actually changed.