grevory / angular-local-storage

An AngularJS module that gives you access to the browsers local storage with cookie fallback
Other
2.83k stars 585 forks source link

JavaScript issue in IE12 or lower #359

Open TemitaTom opened 7 years ago

TemitaTom commented 7 years ago

Hi, some errors in javascript

Versions:

Case: Inside callback function, localStorage plugin call "finally" clause after try catch. This not work in IE 12- For solve this, please use try catch correcly -if is possible- and return in every function a default object if it is fail.

Example:

Current code

// Directly adds a value to local storage // If local storage is not available in the browser use cookies // Example use: localStorageService.add('library','angular'); var addToLocalStorage = function (key, value, type) { var previousType = getStorageType();

    try {
      setStorageType(type);

      // Let's convert undefined values to null to get the value consistent
      if (isUndefined(value)) {
        value = null;
      } else {
        value = toJson(value);
      }

      // If this browser does not support local storage use cookies
      if (!browserSupportsLocalStorage && self.defaultToCookie || self.storageType === 'cookie') {
        if (!browserSupportsLocalStorage) {
          $rootScope.$broadcast('LocalStorageModule.notification.warning', 'LOCAL_STORAGE_NOT_SUPPORTED');
        }

        if (notify.setItem) {
          $rootScope.$broadcast('LocalStorageModule.notification.setitem', {key: key, newvalue: value, storageType: 'cookie'});
        }
        return addToCookies(key, value);
      }

      try {
        if (webStorage) {
          webStorage.setItem(deriveQualifiedKey(key), value);
        }
        if (notify.setItem) {
          $rootScope.$broadcast('LocalStorageModule.notification.setitem', {key: key, newvalue: value, storageType: self.storageType});
        }
      } catch (e) {
        $rootScope.$broadcast('LocalStorageModule.notification.error', e.message);
        return addToCookies(key, value);
      }
      return true;
    } finally {
      setStorageType(previousType);
    }
  };

Working code

// Directly adds a value to local storage // If local storage is not available in the browser use cookies // Example use: localStorageService.add('library','angular'); var addToLocalStorage = function (key, value, type) {

  var previousType = getStorageType();
  var result = false;

  try {

      setStorageType(type);

      // Let's convert undefined values to null to get the value consistent
      if (isUndefined(value)) {
          value = null;
      } 
      else {
          value = toJson(value);
      }

      // If this browser does not support local storage use cookies
      if (!browserSupportsLocalStorage && self.defaultToCookie || self.storageType === 'cookie') {
          if (!browserSupportsLocalStorage) {
              $rootScope.$broadcast('LocalStorageModule.notification.warning', 'LOCAL_STORAGE_NOT_SUPPORTED');
          }

          if (notify.setItem) {
              $rootScope.$broadcast('LocalStorageModule.notification.setitem', {key: key, newvalue: value, storageType: 'cookie'});
          }

          result = addToCookies(key, value);
      }
      else {
          try {

              if (webStorage) {
                  webStorage.setItem(deriveQualifiedKey(key), value);
              }
              if (notify.setItem) {
                  $rootScope.$broadcast('LocalStorageModule.notification.setitem', {key: key, newvalue: value, storageType: self.storageType});
              }
              result = true;
          } 
          catch (e) {

              $rootScope.$broadcast('LocalStorageModule.notification.error', e.message);
              result = addToCookies(key, value);
          }
      }
  }
  catch (e) { 
      result = false;
  }
  finally { 
      setStorageType(previousType);
  }

  return result;

};

thanks

menzow commented 7 years ago

IE supports try..catch..finally since ie6. I think the issue lies within your control flow that handles the result of addToLocalStorage. Could you share your implementation?

I confirmed compatibility with ie9 and ie10 using this fiddle: https://jsfiddle.net/x78novpf/2/.

TemitaTom commented 6 years ago

Hi, yes, try..finally works well but in this case fails inside http calback function only. And some functions with try...finally not all parts return data.