Skip to content
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

IE11에서 scope.$apply 실행시 오류 발생되어 $rootScope.$$phase 조건으로 판단하도록 수정 #34

Closed
wants to merge 2 commits into from

Conversation

qpitlove
Copy link

@qpitlove qpitlove commented Jan 6, 2015

안녕하세요. outsider님
summernote angular version을 애용하고 있는 평범한 개발자 중 한사람입니다.

최근 IE에서 테스트를 하다 페이지 로딩 시 계속적인 angular exception이 나서 원인을 찾아 봤는데요.
Error: [$rootScope:inprog] $digest already in progress

angular의 dirty checking에서 $$phase 조건범위가 해당 scope이 아닌 $rootScope의 처리로 되어야 함을 angular 1.2.28 소스 코드를 통해 알게 되었습니다.

if (ctrl.$viewValue !== value || (value === '' && revalidate)) {
      if (scope.$root.$$phase) {
        ctrl.$setViewValue(value);
      } else {
        scope.$apply(function() {
          ctrl.$setViewValue(value);
        });
      }
    }

확인해 보시고 반영 부탁드리겠습니다.

ps. 많은 도움 받고 있습니다. ^^

@outsideris
Copy link
Member

확인해 보겠습니다.

@outsideris
Copy link
Member

@qpitlove
확인해 보고 있는데 좀더 자세한 정보를 알려주시면 감사하겠습니다.
제가 지금 이슈를 재현 못하고 있는데 IE11에서 페이지만 로드해도 바로 발생하는 것인지 어떤 액션이 필요한 것인지 궁금하고 다른 브라우저에서는 괜찮으신지 궁금합니다.(사용하시는 angular.js와 angular-summernote의 버전도 알려주세요.)

제가 확인해 보고자 하는 것은 현재 $digest already in progress상황은 $digest가 기존의 코드처럼 현재 scope이 아닌 $rootScope에서 발생하면서 제대로 체크가 안되는 것으로 보이는데 이 부분을 수정하려면 항상 $rootScope로 확인을 해야하는것인지 아니면 별도로 현재 코드로 체크가 안되고 IE11에서만 $rootScope에서 $$phase가 충돌하는 경우가 있는건지 확인해 봐야 할듯 합니다.

angular.js에서 이를 안티패턴으로 얘기하고 있어서 좀더 파봐야 할것도 갔고요. 참고하신 1.2.28의 소스 위치도 알려주시면 도움이 될듯합니다.

@qpitlove
Copy link
Author

qpitlove commented Jan 7, 2015

@outsideris
IE11.0.9600.17501
angular ~1.2.28
angular-summernote ~0.3.0
summernote 0.6.0
입니다.

그리고 본문에 추가한 샘플 코드의 위치는 angular.js 파일의 L17147 입니다.

var listener = function(ev){ 
...
 if (ctrl.$viewValue !== value || (value === '' && revalidate)) {
      if (scope.$root.$$phase) {
        ctrl.$setViewValue(value);
      } else {
        scope.$apply(function() {
          ctrl.$setViewValue(value);
        });
      }
    }
};

ngModel에서 input에 대한 이벤트 처리와 관련된 listener 관련 코드 입니다.

FF 34.0.5
Chrome 39.0.2171.95 (64-bit)
Safari 8.0.2
위 3개의 브라우저는 오류없이 동작되는걸로 확인되고 있구요.

angular-summernote.js 에 scope의 phase에서 확인했던 방식은 아래와 같이 console.log로 테스트를 해본 결과값이었습니다.

if (ngModel && ngModel.$viewValue !== newValue) {
          ngModel.$setViewValue(newValue);
          console.log(scope.$$phase, scope.$root.$$phase ); // 결과 null, '$digest'
          if (scope.$$phase !== '$apply' && scope.$$phase !== '$digest' ) {
            scope.$apply();
          }
        }

추측하기로는
directive가 $compile되면서 activate() 내부에서 scope.$appy() 요청을 하게 되는데 그에 앞서 우선선위가 좀더 높은 directive가 먼저 scope.$apply() 요청을 해서 summernote의 scope.$$phase는 null이라도 다른 directive는 '$digest'를 가지고 있고 $rootScope.$$phase 입장에서는 '$digest' 로 판단하지 않나 생각합니다. ( IE만 이런 현상이 있는 걸로 봐서 좀 더 코드를 봐야 알 수 있을 것 같습니다. )

@qpitlove
Copy link
Author

qpitlove commented Jan 7, 2015

보통 scope에 충돌날 경우에 대비해 $evalAsync도 사용하는걸 봤는데요.
아래 코드는 어떨까요?

if (ngModel && ngModel.$viewValue !== newValue) {
    scope.$evalAsync(function(){
        ngModel.$setViewValue(newValue);
        //console.log('$$phase', scope.$$phase, scope.$root.$$phase, newValue);
    });
    //if (scope.$$phase !== '$apply' && scope.$$phase !== '$digest' ) {
    //    scope.$apply();
    //}
}

@outsideris
Copy link
Member

@qpitlove
네 감사합니다. 제가 IE11 환경이 없어서 테스트 환경부터 좀 구성해야 할듯 하네요.

바로 디렉티브 rootScope에 접근하는게 맞는지 잘 모르겠어서...
일단 현재는

  1. 어떤 $digest와 충돌하는가?
  2. 왜 IE11에서만 충돌하는가?

를 파악해 보려고 합니다. 2번 부분을 생각하면 여기가 아니라 angular.js쪽의 문제가 아닌가 의심되기도 하는데 좀더 테스트해봐야 알 수 있을듯 합니다. angular.js의 버그가 아니라면 1번 부분을 파악해서 코드 주신대로 우회하는 방법을 사용하거나 아니면 $apply()접근 자체를 바꾸어야 할지를 고민해 보려고 합니다.

@qpitlove
Copy link
Author

qpitlove commented Jan 7, 2015

네 감사합니다.
참고로 제가 진행하는 프로젝트의 angular version을 1.3.8로 업데이트 시켜보았고 IE11에서 동일한 문제가 발생되고 있습니다.

1.2.28에 비해 코드가 상당히 바뀌었는데요.
var listener에서 scope.$root.$$phase 조건을 보고 $setViewValue 호출하는 로직이 변경되었습니다.
L19612

var listener = function(ev) {
    if (timeout) {
      $browser.defer.cancel(timeout);
      timeout = null;
    }
    if (composing) return;
    var value = element.val(),
        event = ev && ev.type;

    // By default we will trim the value
    // If the attribute ng-trim exists we will avoid trimming
    // If input type is 'password', the value is never trimmed
    if (type !== 'password' && (!attr.ngTrim || attr.ngTrim !== 'false')) {
      value = trim(value);
    }

    // If a control is suffering from bad input (due to native validators), browsers discard its
    // value, so it may be necessary to revalidate (by calling $setViewValue again) even if the
    // control's value is the same empty value twice in a row.
    if (ctrl.$viewValue !== value || (value === '' && ctrl.$$hasNativeValidators)) {
      ctrl.$setViewValue(value, event);
    }
  };

참고 부탁 드리겠습니다.

@outsideris outsideris added the bug label Jan 7, 2015
@outsideris outsideris self-assigned this Jan 7, 2015
@outsideris
Copy link
Member

IE 11에서 이슈를 재현해보려고 했는데 재현을 못해서 확인을 못하고 있습니다.

http://jsfiddle.net/outsider/n8dt4/139/embedded/result%2Chtml%2Cjs%2Ccss/ 여기서 말씀하신 버전으로 맞추고 하단의 "code : HTML string in summernote" 부분의 예제가 ng-model을 사용하고 있어서 이부분에서 수정을 해보았는데 IE 11에서(언급하신 버전과 동일 버전입니다.) 오류가 발생안하고 있습니다.

위 예시에서 어떻게 재현할 수 있는지 확인 부탁드립니다.

@qpitlove
Copy link
Author

@outsideris 죄송합니다. 다른 작업때문에 답변이 많이 늦었습니다.
보내주신 URL로 다시 확인해보도록 하겠습니다.
감사합니다.

@qpitlove
Copy link
Author

보내주신 샘플에서 ng-model로 설정된 부분의 예제가 제 IE11에서도 정상동작되고 error 없음을 확인했습니다.
저희가 작업하고 있는 페이지가 full spa로 되어 있는터라 제가 직접 원인 파악을 하나씩 해봐야 할 것 같은데요.
( angular 초기화 시점부터 수많은 API와 async 로직 그리고 사용하고 있는 오픈 소스 angular 모듈이 무진장 많아서요 ^^; )
확인되는데로 알려 드리도록 하겠습니다.

감사드립니다.

@outsideris
Copy link
Member

네 업무도 있으신데 너무 급하게 확인하진 않으셔도 됩니다. ^^

@qpitlove
Copy link
Author

@outsideris 답변 감사드립니다.
빠른 시일내에 오류 발생 샘플도 작성해서 url 알려드리겠습니다.

@qpitlove
Copy link
Author

안녕하세요. 답변이 많이 늦었습니다.
아래 IE11에서도 초기 페이지 로딩시 유사한 스크립트 오류가 나도록 샘플 페이지 데모 URL 첨부해 드립니다. ( 실제로는 엮이는 부분이 너무 많아서 Exception tracking 에 대한 걸 기준으로 작성했습니다.)
http://plnkr.co/edit/NNvkuA1jKufGudz4Y9rd?p=preview

간단히 요약하자면
하나의 에디터가 특정 위치에 고정이 아닌 DOM의 위치를 옮겨다닐수 있음. (위키 본문 수정과 유사)

  1. 목록중 하나의 아이템 영역 클릭시 에디터 창으로 대체되고 수정 후 원래 있던 위치로 DOM 이동이 됨
  2. 특정 아이템 위치에서 수정중일때 해당 영역을 리프레시할 경우가 있으면 ( 서비스 컨트롤러의 $scope.$watch... ) 에디터의 현재 작업을 멈추고 초기화 후 원래 최초에 있던 위치로 DOM 이동이 됨

재미있는 부분은 jQuery의 $(...).append( editorWrapElement ) 이동시 summernote 에서 onChange에 대한 이벤트를 실행 시켜주는데 이때 angular-summnernote의 updateModel() 메소드가 실행된다는 점입니다. ( updateModel() 은 $scope.$apply 를 실행시켜 줍니다. )

그리고 $scope.$watch내에 실행된 상태는 $rootScope.$$phase 가 $digest 인 상태이며 여기서 $apply가 실행된다면 당연히 Exception이 발생될 수 밖에 없을 것 같습니다. (exception 발생조건은 $rootScope 만을 참고로 합니다.)
https://github.com/angular/angular.js/blob/master/src/ng/rootScope.js#L1283

https://github.com/angular/angular.js/blob/master/src/ng/rootScope.js#L1271
https://github.com/angular/angular.js/blob/master/src/ng/rootScope.js#L202
그리고 angular-summernote인 경우 isolate-scope 형태로 $rootScope과는 다르게 $apply함수를 prototype 상속 하지 않을것 같은데요.
이경우 $$phase 도 따로 설정될 것 같습니다.

참고 부탁 드리겠습니다.

@outsideris
Copy link
Member

이슈 추적하느라고 고생이 많으셨겠네요.
조만간 내부에서 처리 가능한 이슈인지 확인해 보겠습니다.

@qpitlove
Copy link
Author

네 감사합니다. ^^

outsideris added a commit that referenced this pull request Feb 7, 2015
@outsideris
Copy link
Member

@qpitlove
일이 바빠서 확인이 늦었습니다.
문서를 찾아보니 $apply대신 $timeout으로 $digest의 충돌을 피할 수 있어서 $timeout으로 수정했습니다.
올려주신 예제에서 $timeout 버전으로 바꾸었을 때는 오류가 발생안하는데 혹시 이 수정버전으로 문제가 해결되었는지 확인해 주시면 감사하겠습니다.

@qpitlove
Copy link
Author

@outsideris 네 확인해 보고 다시 말씀드리겠습니다.

@qpitlove
Copy link
Author

@outsideris 정상적으로 동작확인되었고
bower install 가능하도록 버전업(현 v0.3.1) 시켜주시면 반영해보도록 하겠습니다.
감사합니다.

@outsideris
Copy link
Member

fixed in v0.3.2

@outsideris outsideris closed this Feb 13, 2015
@outsideris
Copy link
Member

적용해서 버전 올렸습니다. 미처 고려하지 못한 이슈에 적극적으로 대응해 주셔서 감사합니다.

@qpitlove
Copy link
Author

@outsideris 감사드립니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants