angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.85k stars 27.52k forks source link

how i can remove the comments created by ng-if and ng-repeat? #8722

Closed cc17 closed 8 years ago

cc17 commented 10 years ago

Is there a way to prevent Angular from creating "helper" HTML comments? For example,

<div ng-include="myTemplate"></div> Will transform into something like

<!-- ngInclude: 'hurr-durr.html' -->
<div ng-include="myTemplate"></div>

How do I stop this?

Narretz commented 10 years ago

I can't tell you how it works internally, but angular needs these comments to keep track of directives that may or may not have actual DOM nodes as output. For example, when ngIf is false, there's no DOM node, but the angular compiler needs the comment to know at which position in the tree the directive is located. I'm sure someone else, for example @caitp can explain this better.

btford commented 10 years ago

@cc17 why do you want to get rid of these elements? There's likely a better way to achieve your goal.

cc17 commented 10 years ago

the comments will show some product logic that i don't want other people see.

2014-08-26 7:05 GMT+08:00 Brian Ford notifications@github.com:

@cc17 https://github.com/cc17 why do you want to get rid of these elements? There's likely a better way to achieve your goal.

— Reply to this email directly or view it on GitHub https://github.com/angular/angular.js/issues/8722#issuecomment-53349716.

ghost commented 10 years ago

I agreed with @cc17. Even I want to achieve the same.

lgalfaso commented 10 years ago

@cc17 I would like to understand this. Angular needs these comment nodes to be present for reasons that are outside of this topic. Now, you said

the comments will show some product logic that i don't want other people see

Are you saying that

If you are talking about the first option, then I think it might be a stretch but there is some chance that an opt-in can be added. If you are talking about the second option, then removing them would involve a big refactor on how directives transclusion works and I doubt that will happen soon.

DigitalSmile commented 10 years ago

@lgalfaso

You would like that the comment nodes do not show any information? I.e if these were empty comment nodes then that would be ok

I could say that would be an option.

ghost commented 10 years ago

@lgalfaso yes first option fine. Expose essential comment body that will not break present directives behavior.

seavor commented 9 years ago

I came here looking for the same answers to the same problem, but for different reasons (it was just really annoying debugging the DOM elements with a million comments in the way).

Listening to other people describe Angular's dependency on the comments, it makes sense. To add my two cents on your concern of hiding app logic.. Angular is javascript, and as we all know, there's no way to really secure your javascript code from users who really want to peak in and see what you got under the hood. Removing the comments, in my opinion, would only keep casual peekers from getting a look at your logic. It won't deter an attacker, and i'd be highly surprised if an attacker would even consider your angular comments as the route to finding a possible exposure or weakness in your code.

frfancha commented 9 years ago

@cc17 "the comments will show some product logic that i don't want other people see." ? To see the comments you must open the dev tools of the browser. There, ALL your (front end part) product logic is shown! Your html (original and current), javascript, network requests...

frfancha commented 9 years ago

@seavor "it was just really annoying debugging the DOM elements with a million comments in the way" I wonder which angular page (with an acceptable response time ;-) ) you can create generating "a million comments"

F1LT3R commented 8 years ago

+1

I would like to see an option to remove comments from the live HTML too. To my eyes it is ugly and distracting. Unlike my text editor the developer console is 1/3 of the page high, and so these extra comments really bug.

Are there more details on why exactly Angular needs these?

Narretz commented 8 years ago

Ugly and distracting is not a very powerful reason. 😉 The comments are used by angular as markers for content that will be inserted there. For example ngIf elements that are not displayed need this. So I don't think there is a realistic chance that they will ever be removed. Angular 2 should theoretically have the same issue but maybe it's different there. Am 10.02.2016 19:16 schrieb "Alistair G MacDonald" <notifications@github.com

:

+1

I would like to see an option to comments removed from the live HTML too. To my eyes it is ugly and distracting.

Are there more details on why exactly Angular needs these?

— Reply to this email directly or view it on GitHub https://github.com/angular/angular.js/issues/8722#issuecomment-182509708 .

gkalpak commented 8 years ago

Yes, ng2 has comments for structural directives too. (I think they were using <script> elements, but switch to comments at some point, because the elements were breaking CSS selectors iirc.)

F1LT3R commented 8 years ago

Yes understood @Narretz, ugliness may not be hugely motivating, but it's unclear what the actual tradeoff was/is. Does it perform better by using existing DOM nodes? I can't image that would make much of a difference (maybe I'm way off with that). Or was it just easier to iterate through the DOM nodes rather than create a JavaScript array?

gkalpak commented 8 years ago

@F1LT3R, the comment nodes are there as markers. They are necessary for the case when the contents are not shown (e.g. an ngIf expression evaluating to false, an ngSwitchWhen value not matched by the current ngSwitch expression etc). In that case, we need a placeholder in the DOM to know where to insert the actual elements later (e.g. when the ngIf expression evaluates to true etc).

teknosains commented 8 years ago

I hope there will be an Option for that.

Leaving comment like this I think its not a good idea <!-- ngIf: transaction.status==9 && transaction.status!==8 -->

gkalpak commented 8 years ago

@codetrash, if you have a better idea to propose, we're all ears :smiley:

Narretz commented 8 years ago

@codetrash why is it not a good idea?

teknosains commented 8 years ago

@Narretz for a quite sensitive application like Banking, Payment service etc...leaving this

!-- ngIf: transaction.status==9 && transaction.status!==8 -->

could lead into a risky situation.

@gkalpak maybe Leaving an encrypted marker etc. Do you have any?

Narretz commented 8 years ago

@codetrash The expression markup is based on your javascript code which is exposed the user agent anyway. Obfuscating your Javascript code may make it more difficult to get the information, but your application logic can still be extracted. We could of course try to remove / obfuscate the expression, but I personally think there's no security risk here and it's low priority. If someone wants to give it a shot, you are very welcome, although I can't guarantee that it will be merged.

F1LT3R commented 8 years ago

Thanks @gkalpak, makes sense. I could see why attempting to track the DOM changes in JS might be quite impractical for performance and maintainability. I suppose if we could say "Angular controls all things in the DOM" it might be somewhat more feasible, but Angular tends to get mushed together with all kinds of 3rd-party libraries so... eh. The "lesser of two evils".

frfancha commented 8 years ago

@codetrash <<leaving !-- ngIf: transaction.status==9 && transaction.status!==8 --> could lead into a risky situation>> ?? This code is nothing more that you can read in the html sent in clear text at first stage anyway, so ...

smurugavel commented 8 years ago

+1.

Narretz commented 8 years ago

@smurugavel Please don't simply +1 this issue. There's still no compelling case against these comments, so simply +1 will not make us implement this.

smurugavel commented 8 years ago

@Narretz , like @cc17 said, business logic was my concern, that need not be seen by others who can see the source code through developer tools. going through the suggestions in this thread, @lgalfaso 's first option was OK for me. Alternatively, can angular team encrypt these comments that a human cannot read or add a unique identifier that angular can track internally? just my thoughts..

gkalpak commented 8 years ago

@smurugavel, as already mentioned in this thread, these comments do not contain anything that is not already visible as plain text in your templates. Whoever can see your source code can see this business logic. Getting rid of the comments won't solve your problem.

ManishLSN commented 8 years ago

I hope there will be an Option for remove and get this again on filteration.

Leaving comment like this I think its not a good idea

What else if i have to apply filter and use ul li again at this time my commented li value gets appear in view again.

gkalpak commented 8 years ago

@ManishLSN, not sure what you mean. There is no commented li. Just a plain HTML comment. That said, I believe it would make sense to have empty comments when debug info is disabled (the contents of the comments do not serve any other purpose than "debugability").

WDYT @Narretz, @lgalfaso ?

Again (for people that raised this concern), this will have no impact on security.

petebacondarwin commented 8 years ago

We need comments nodes to be in the DOM or Angular simply will not work. We can implement the idea of removing the text of the comment if debugInfo is disabled. This is a fairly straightforward change. PRs anyone?

smurugavel commented 8 years ago

Thanks !!!

jdforsythe commented 8 years ago

@codetrash This is closed and I think it's good for "hiding" the logic from the inspector. But as stated before, your plain HTML and JS are served to the browser, so it shouldn't be considered in any way to be more secure. This could likely be protected much better by using server-side angular with 2.0. In any case, the only "risky situation" I could see is if they could change values in the inspector and either view or save data they shouldn't be able to - but this could be stopped by not providing that data to the model in the first place. It should be considered bad practice to send sensitive data from the server if it should be hidden from the user - in other words, permissions should be enforced server-side, especially for banking, payment service, etc. Anything sent to the browser should be considered publicly available.

F1LT3R commented 8 years ago

That's an interesting use case Akuno. I can set why that would be frustrating.

I think in practice you would want to filter out certain elements even in a non Angular scenario. I frames, comments, etc.

More work to implement, yes, but a cleaner more controllable pipeline between one format and another is probably a good idea. On Mar 25, 2016 2:55 AM, "akunno" notifications@github.com wrote:

I hope I'm understanding this correctly, because I'm in a similar boat.

Currently my dom contains a lot of comments and images like this:

when I call innerHTML on the element's parent, I end up with something like this:

I am unaware of a way to extract the HTML without the angular directives. I'm trying to extract the rendered HTML without angular because I'm passing this to a converter which creates a word doc out of it. Currently the word doc contains a bunch of image tags with red X's because it's still trying to render the image as if it was an image. The comments are also massively increasing the size of the string.

I've attempted to remove the comments, but I still cannot work out a way to extract the rendered HTML without angular directives polluting it. Ideally, when I call innerHTML I wanted to see if the ngIf was true or alternatively nothing, if ngIf returned false.

I hope I'm understanding this correctly, and that this is a +1 for a way to somehow extract the rendered output without comments/directives.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/angular/angular.js/issues/8722#issuecomment-201172995

mashpie commented 8 years ago

another use case: at least skipping newlines by config for css:empty pseudo to get working https://css-tricks.com/almanac/selectors/e/empty/

deusdeorum0 commented 8 years ago

@mashpie came across the same use case just now, what a coincidence

petebacondarwin commented 8 years ago

Angular does not add newlines around the marker comments. See http://plnkr.co/edit/z1rJZd7yU0TYZmDAynTh?p=preview

RHanmant commented 6 years ago

You can do that by using.

app.config(['$compileProvider', function ($compileProvider) { // disable debug info $compileProvider.debugInfoEnabled(false); }]);

Narretz commented 6 years ago

@RHanmant this doesn't remove the comments completely, it just removes variable / property names from the comments. The comments are necessary.

AndersBillLinden commented 5 years ago

I came to this issue report when trying to use a css rule like #mydiv > div:last-child and the div that should be the last child was not because of the angularjs comment afterwards.

Narretz commented 5 years ago

CSS selectors ignore comments, so what you described cannot happen.

badeamihai commented 4 years ago

I hope there will be an Option for that.

Leaving comment like this I think its not a good idea <!-- ngIf: transaction.status==9 && transaction.status!==8 -->

Yes, everyone will know the dev was too lazy to add some constants instead of hardcoding values :). And yes, there is a bit of disclosure ( because it's on the frontend ), but if you sanitized the backed thoroughly, nobody will exploit this "leaked" info. So i think we should focus on that part.

chriswoodie commented 2 years ago

I have a scenario where I check the content of an element in order to do things, but it treats the comments as being the actual content, so the app breaks because of it. So now I will probably have to force other developers to manually set a boolean to say if the element has content or not, instead of just being able to check it directly. Or remove the comments before checking the content.

Either option is annoying.

Narretz commented 2 years ago

@skog-newglue but if you are checking for content, why not check for node type, too? node.nodeType === Node.COMMENT_NODE ...

Anyway, you shouldn't be using angular.js anymore. It's totally unsupported.

chriswoodie commented 2 years ago

@Narretz Ah didn't realise this was posted in AngularJS, it's still an issue in the latest Angular though, which I'm using. Not sure if I can do that since I'm getting the content with innerHTML, which gives you a string.