Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngSanitize can make input placeholder modiafiable in Internet Explorer 11 #15700

Closed
floftar opened this issue Feb 10, 2017 · 6 comments
Closed

ngSanitize can make input placeholder modiafiable in Internet Explorer 11 #15700

floftar opened this issue Feb 10, 2017 · 6 comments
Labels
browser: IE11 resolution: invalid

Comments

@floftar
Copy link
Contributor

floftar commented Feb 10, 2017

Do you want to request a feature or report a bug?

Report a bug.

What is the current behavior?

ngSanitize can make input placeholder modiafiable in Internet Explorer 11 if input element's parent has disabled attribute. User can click input and start modifying placeholder text. Without ngSanitize input is simply disabled in Internet Explorer 11.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

<!DOCTYPE html>
<html ng-app="customApp">
  <head>
    <title>Internet Explorer 11 and input placeholder</title>
  </head>
  <body disabled>
    <input type="text" size="60" placeholder="Placeholder can be modified in Internet Explorer 11">

    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.6.1/angular.min.js"></script>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.6.1/angular-sanitize.min.js"></script>
    <script>

    angular.module('customApp', ['ngSanitize']);

    </script>
  </body>
</html>

What is the expected behavior?

Input with placeholder would behave identically as plain HTML without AngularJS (i.e., input is simply disabled).

Which versions of AngularJS, and which browser / OS are affected by this issue? Did this work in previous versions of AngularJS? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

AngularJS: 1.6.1, 1.6.2 and snapshot (build.5274+sha.79c4a71)
Internet Explorer 11 - versions 11.0.9600.18524 and 11.0.9600.18537
@Narretz
Copy link
Contributor

Narretz commented Feb 10, 2017

Hi,
there are a few things going on here, but essentially, this is a bug in IE11 and has nothing to do with AngularJS. You can remove ngSanitize and AngularJS completely and you will see the same behavior.

Also, putting disabled on the body is not suppowed to disable the children that are input elements. No other browser does anything when you do that. It looks like IE has a bug there.

in conclusion, it's a bug and if don't put disabled on parent elements, and this should not affect you.

@Narretz Narretz closed this as completed Feb 10, 2017
@floftar
Copy link
Contributor Author

floftar commented Feb 11, 2017

Hi,

I agree that Internet Explorer 11 behaves strange. However if I change line with angular.module in my above example to:

angular.module('customApp', []);

Placeholder cannot be anymore modified. At least for me and that is why I thought it has to do something with AngularJS.

Using disabled in body was for providing a minimal example. You could for example have disabled as binding in your component:

<!DOCTYPE html>
<html ng-app="customApp">
  <head>
    <title>IE 11 and input placeholder</title>
  </head>
  <body>
    <custom-input disabled="true"></custom-input>

    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.6.1/angular.min.js"></script>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.6.1/angular-sanitize.min.js"></script>
    <script>

    angular.module('customApp', ['ngSanitize']).component('customInput', {
      bindings: {
        disabled: '<'
      },

      template: '<fieldset ng-disabled="$ctrl.disabled"><input type="text" placeholder="Placeholder"></fieldset>'
    });

    </script>
  </body>
</html>

In above example you get similar behavior with Internet Explorer 11 and Chrome 56 but placeholder text can be modified in Internet Explorer. However behavior is same for both browsers if ngSanitize is not used:

angular.module('customApp', []).component('customInput', {...

@Narretz
Copy link
Contributor

Narretz commented Feb 11, 2017

Ok, I was testing in IE11 11.576.14393.0, Update Version 11.0.38 and there was no difference with / without ngSanitize. Your and my version numbers looks quite different, so I can't say which was is newer.

It's also possible that I didn't correctly translate your example to this plnkr which I used to test: http://plnkr.co/edit/jDJv5NtwwDSsp9hRT2rf?p=preview Can you check if this fails for you in IE11?

It's always better to provide a runnable example instead of just the code.

That said, I don't think this is something we can / should fix, since the workaround is simple.

@floftar
Copy link
Contributor Author

floftar commented Feb 11, 2017

Thank you for example code. Internet Explorer behaves as I described earlier, but there is a catch. If I run the Plunker as such it does not work. You have to open the resulting iframe (right side, http://run.plnkr.co/...) in the browser and then you can see the behavior I described. At least in my Internet Explorer. I have no idea why the app cannot be in iframe.

Behavior can be changed by modifying app.js line 1:

var app = angular.module('plunker', ['ngSanitize']);

vs

var app = angular.module('plunker', []);

I agree that this is probably something that is not even worth trying to fix, but I wanted to bring this up just in case. My latter example is maybe a bit more concrete where somebody could use disabled as custom element attribute and might be surprised how Internet Explorer works.

@Narretz
Copy link
Contributor

Narretz commented Feb 15, 2017

FWIW, there's no change for me when I switch to run.plnkr ...

@floftar
Copy link
Contributor Author

floftar commented Feb 18, 2017

Thanks for investigating this problem.

@gscoppino gscoppino mentioned this issue Mar 14, 2018
Merged
3 tasks
Narretz pushed a commit that referenced this issue Mar 19, 2018
@gscoppino @Narretz
docs(guide/Internet Explorer Compatibility): include warnings for usa…
c68b31c 
…ge of 'disabled' attribute

* docs(guide/Internet Explorer Compatibility): Mention 'disabled' attribute

Setting the 'disabled' attribute on an element that has descendant elements has unexpected behavior in Internet Explorer 11.

* Input elements that are descendants have the text content of the 'placeholder' attribute inserted as the value (and it is not removed when typing in the field).
* Select elements that are descendants are disabled.

To avoid this issue, it is important to not set `disabled` or `ng-disabled` on an element that has descendant form elements. Normally these should only be used on actual form controls so the issue would not manifest.

The issue can also appear if a directive/component is named 'disabled' or takes an attribute named 'disabled' as an input/output attribute, so avoid these.

Closes  #16490
Related #15700
Narretz pushed a commit that referenced this issue Mar 19, 2018
@gscoppino @Narretz
docs(guide/Internet Explorer Compatibility): include warnings for usa…
24ddbaf 
…ge of 'disabled' attribute

* docs(guide/Internet Explorer Compatibility): Mention 'disabled' attribute

Setting the 'disabled' attribute on an element that has descendant elements has unexpected behavior in Internet Explorer 11.

* Input elements that are descendants have the text content of the 'placeholder' attribute inserted as the value (and it is not removed when typing in the field).
* Select elements that are descendants are disabled.

To avoid this issue, it is important to not set `disabled` or `ng-disabled` on an element that has descendant form elements. Normally these should only be used on actual form controls so the issue would not manifest.

The issue can also appear if a directive/component is named 'disabled' or takes an attribute named 'disabled' as an input/output attribute, so avoid these.

Closes  #16490
Related #15700
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
browser: IE11 resolution: invalid
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Narretz @floftar