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

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

Closed
cc17 opened this issue Aug 22, 2014 · 43 comments
Closed

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

cc17 opened this issue Aug 22, 2014 · 43 comments

Comments

@cc17
Copy link

cc17 commented Aug 22, 2014

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
Copy link
Contributor

Narretz commented Aug 22, 2014

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
Copy link
Contributor

btford commented Aug 25, 2014

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

@btford btford added this to the Purgatory milestone Aug 25, 2014
@cc17
Copy link
Author

cc17 commented Aug 26, 2014

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
#8722 (comment).

@ghost
Copy link

ghost commented Aug 26, 2014

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

@lgalfaso
Copy link
Contributor

@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

  • 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
  • The comment nodes should not be there at all

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
Copy link

@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
Copy link

ghost commented Sep 15, 2014

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

@seavor
Copy link

seavor commented Dec 27, 2014

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
Copy link

@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
Copy link

@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
Copy link

F1LT3R commented Feb 10, 2016

+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
Copy link
Contributor

Narretz commented Feb 10, 2016

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
#8722 (comment)
.

@gkalpak
Copy link
Member

gkalpak commented Feb 10, 2016

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
Copy link

F1LT3R commented Feb 10, 2016

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
Copy link
Member

gkalpak commented Feb 10, 2016

@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
Copy link

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
Copy link
Member

gkalpak commented Feb 16, 2016

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

@Narretz
Copy link
Contributor

Narretz commented Feb 16, 2016

@codetrash why is it not a good idea?

@teknosains
Copy link

@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
Copy link
Contributor

Narretz commented Feb 16, 2016

@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
Copy link

F1LT3R commented Feb 16, 2016

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
Copy link

@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
Copy link

+1.

@Narretz
Copy link
Contributor

Narretz commented Feb 20, 2016

@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
Copy link

@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
Copy link
Member

gkalpak commented Feb 23, 2016

@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
Copy link

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
Copy link
Member

gkalpak commented Feb 24, 2016

@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
Copy link
Member

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?

lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Feb 25, 2016
…bled is false

When debugInfoEnabled is `false` when comments generated by transclusions, ngIf,
ngRepeat and ngSwitch will not contain any information about the directive nor
the expression associated with it.

Closes: angular#8722
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Feb 25, 2016
…bled is false

When debugInfoEnabled is `false` when comments generated by transclusions, ngIf,
ngRepeat and ngSwitch will not contain any information about the directive nor
the expression associated with it.

Closes: angular#8722
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Feb 25, 2016
…bled is false

When debugInfoEnabled is `false` when comments generated by transclusions, ngIf,
ngRepeat and ngSwitch will not contain any information about the directive nor
the expression associated with it.

Closes: angular#8722
lgalfaso added a commit to lgalfaso/angular.js that referenced this issue Feb 25, 2016
…bled is false

When debugInfoEnabled is `false` when comments generated by transclusions, ngIf,
ngRepeat and ngSwitch will not contain any information about the directive nor
the expression associated with it.

Closes: angular#8722
lgalfaso added a commit that referenced this issue Feb 25, 2016
…bled is false

When debugInfoEnabled is `false` when comments generated by transclusions, ngIf,
ngRepeat and ngSwitch will not contain any information about the directive nor
the expression associated with it.

Closes: #8722
@smurugavel
Copy link

Thanks !!!

@jdforsythe
Copy link

@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
Copy link

F1LT3R commented Mar 26, 2016

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:

ng-src="data:image/gif;base64,....">

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
#8722 (comment)

@mashpie
Copy link

mashpie commented Jun 3, 2016

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
Copy link

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

@petebacondarwin
Copy link
Member

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

@RHanmant
Copy link

You can do that by using.

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

@Narretz
Copy link
Contributor

Narretz commented Jan 24, 2018

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

@AndersBillLinden
Copy link

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
Copy link
Contributor

Narretz commented Jun 25, 2019

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

@badeamihai
Copy link

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
Copy link

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
Copy link
Contributor

Narretz commented Mar 29, 2022

@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
Copy link

@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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.