Allow html in title attributes in tab-pane directive

[jmaynier]

It is common to add icons in tab title, usually with while using bootstrap.
So I think the default template should allow this, either using ng-bind-html-unsafe or ng-bind-html with ngSanitize, 

[pkozlowski-opensource]

@jmaynier thnx for your contribution! Would you mind adding unit tests?

[petebacondarwin]

I am not that keen on this unsafe binding. It could be a potential security flaw.

Is there not a better way to work this? I was looking at specialized transclusion earlier. I believe this is a use case.

[pkozlowski-opensource]

@petebacondarwin indeed, would be cool if we could limit the subset of tags to be used in a title. We could create a specialized filter for this maybe?

[ajoslin]

Sure, someone could put <pane heading="'<script>window.location="google.com"</script>'">, or <pane heading="someInsecureRemoteData">, but IMO the developer should worry about someInsecureRemoteData, not us.

[petebacondarwin]

But, without looking at the code, the developer wouldn't necessarily know that the title/heading binding is being displayed as an unsafe binding. If you want to put the responsibility on the developer then the attribute should be called unsafe-html-title.

[jmaynier]

Why not have 2 attributes, title as it works now and html-title that would be used in an ng-bind-html in the template. The use of html-title would require the inclusion of ngSanitize ?
So the dev has the choice to use the regular title without adding extra js script, or use the html version and include ngSanitize.

[ajoslin]

@jmaynier That sounds like a fair idea.

[pkozlowski-opensource]

@ajoslin @jmaynier I'm not sure but from what I understand we can't conditionally include a module (ngSanitize in this case). So the problem is that if we would require presence of the $sanitize in a directive we would have to force people to include ngSanitize anyway...

I'm not excited about the idea of using ng-bind-html-unsafe nor forcing people to include ngSanitize...

I hope I'm missing an obvious solution here as having some basic HTML it a title (b, i, etc.) sounds like a good idea...

[petebacondarwin]

I think if you are going to pass HTML to a directive it should be via a
child element of the directive that will be transcluded. Safest and allows
complex compiled functionality to appear, such as directives.

On 25 January 2013 18:58, Pawel Kozlowski [email protected] wrote:

@ajoslin https://github.com/ajoslin @jmaynierhttps://github.com/jmaynierI'm not sure but from what I understand we can't conditionally include a
module (ngSanitize in this case). So the problem is that if we would
require presence of the $sanitize in a directive we would have to force
people to include ngSanitize anyway...

I'm not excited about the idea of using ng-bind-html-unsafe nor forcing
people to include ngSanitize...

I hope I'm missing an obvious solution here as having some basic HTML it a
title (b, i, etc.) sounds like a good idea...

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/67#issuecomment-12715312.

[pkozlowski-opensource]

@petebacondarwin I hear what you are saying but how this would work in practice? Would it require an additional element (pane-title?) or something? I just wonder if this wouldn't create HTML API that is a bit verbose. Anyway, the thing is that I'm not seeing at the moment how the markup could look like.

If we can't come up with a nice syntax I would be leaning toward using ng-bind-html-unsafe and warning people about sanitizing variables if needed. It would be then to users if this directive to sanitize (or not) titles. Once again, I'm not big fan of it but searching for a practical solution here.

[petebacondarwin]

Similar to HTML4

or HTML5

and

On 26 January 2013 21:04, Pawel Kozlowski [email protected] wrote:

@petebacondarwin https://github.com/petebacondarwin I hear what you are
saying but how this would work in practice? Would it require an additional
element (pane-title?) or something? I just wonder if this wouldn't create
HTML API that is a bit verbose. Anyway, the thing is that I'm not seeing at
the moment how the markup could look like.

If we can't come up with a nice syntax I would be leaning toward using
ng-bind-html-unsafe and warning people about sanitizing variables if
needed. It would be then to users if this directive to sanitize (or not)
titles. Once again, I'm not big fan of it but searching for a practical
solution here.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/67#issuecomment-12742250.

[pkozlowski-opensource]

@petebacondarwin Peter, I can see the general idea but could you propose the exact syntax? What I'm afraid of that the syntax might get so verbose that it might defeat the whole purpose of this directive. At least the proposals I was coming up with were really verbose...

[ajoslin]

So we have two options:

1. Pane with heading and heading-html-unsafe attribute, user chooses:

 <pane heading-html-unsafe="<i class='icon-book'></i> {{title}}"></pane>

2. Pane with heading attribute, or optional pane-heading child directive:

<pane>
  <pane-heading><i class="icon-book"></i> {{title}}</pane-heading>
</pane>

I really kind of like the child element directive like @petebacondarwin said. It's verbose, but it works.

[petebacondarwin]

I am really not keen on html inside attributes!

It is actually quite easy to implement the <pane-heading> element You just instantiate the transcluded elements in the link function, pull out the element, and pass this up to the tab directive controller.

[pkozlowski-opensource]

OK< based on the discussion we will skip merging of this PR. Opened an issue to add support for HTML in titles:
#124

@jmaynier thnx for this PR but we would like to have slightly different solution. Please feel free to open another PR!