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

content of script type="text/ng-template" shoul be parsed as html #228

Closed
michal-minich opened this issue May 23, 2013 · 6 comments
Closed
Milestone

Comments

@michal-minich
Copy link

Angular uses script tag with type text/ng-template as inline html template
see: http://docs.angularjs.org/api/ng.directive:script

vibe.d currently handles all script tag content as CDATA. Whole ouput of script tag is uparsed sent to browser. This prevents this usage of cript tag.

vibe.d should for all script of type="text/ng-template" parse and evalute content as normal html

@s-ludwig
Copy link
Member

s-ludwig commented Apr 2, 2014

Maybe the CDATA wrapper should only be output for :javascript after all and never for a simple script tag, even for JavaScript. It seems that Jade also just outputs the plain text contents.

@michal-minich
Copy link
Author

As I undertand how vibe.d interprets the templates (which might not be correct):

  • it interprets body of the template as tree structure - specifically in intention to convert it to html.
  • html script tag (and also style which is not under discussion here) can contain non-html content - so vibe decides to not parse it - and just output the entire content as it is - (as plain text/cdata).

One solution is this - under assumption that content needs to be parsed in order to be interpreted as html:

  • thread content [script type="text/ng-template"] specially from any other [script (...)] tag - if script has type="text/ng-template" attribute - consider that content is jade tree and parse and interpret it (i.e. same as div/span tags). For script tags other than type="text/ng-template" there will be no change.

This is specific solution to allow AngularJS to be usable (Otherwise the vibe is quite liming the usage of AngularJS). It is not solution for other frameworks that use script tag in similar way, but at the end, I think it will be easy to add the same exception for other values of 'type' attribute.

Btw. if jade do not processes the [script type="text/ng-template"] as html, it is also doing it wrong.

@s-ludwig
Copy link
Member

s-ludwig commented Apr 2, 2014

Ah right, I forgot that script/style still uses the old behavior to treat it's contents as text. The latest Diet compiler supports block text using a trailing dot (#510):

p.
  Hello, World!
  This is text.

Jade also treats script like any other tag since a while and parses its contents as HTML if the trailing dot is missing. AFAIK they made a hard cut and just removed the legacy behavior (breaking all projects that relied on it). The question which is the best deprecation path, because it would be nice to at least not break the majority of projects.

@michal-minich
Copy link
Author

  1. break compatibility
    script
    span // it is parsed as html
    script.
    span // it is parsed as plain text

to update the code after updating the vibe, user would need to add "." after each script and style tag (might be able to do global text replace, but maybe not with 100% automatic accuracy because they are common words)

  1. keep compatibility
    script
    span // it is parsed as plain text
    script.
    span // it is parsed as plain text
    script<
    span // it is parsed as html

using option 2 you will need to introduce additional modifier "<" - or other character you wish, to let the user specify that inner content is html. There is now at least one modifier now - ".", so this will mean additional complexity - now you will have 2 modifiers overriding each other, where only one (".") would be enough

the decision is up to you.

@s-ludwig
Copy link
Member

s-ludwig commented Apr 2, 2014

What I was thinking was something like keeping the old behavior only for style with either no type or type="text/css", but output a warning to add a ".". For all other types, just switch to the new behavior (assuming that >90% of the script tags will be normal CSS). Then after a while both, the old behavior and the warning would be removed.

@michal-minich
Copy link
Author

To be 100% you would need to add warning to all usages of script/style tags. But give that they are all currently not parsed, I cannot imagine any situation where your proposal would cause any trouble in practice. I think it is very good to have gradual transition.

@s-ludwig s-ludwig added this to the 0.7.21 milestone May 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants