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

Lines following a -# comment within a <script> tag disappear after upgrade to 0.6.1 #19

Closed
paulkoegel opened this issue Oct 16, 2013 · 12 comments

Comments

@paulkoegel
Copy link

HAML code:

  %body
    %script{type: 'text/javascript'}
      BB.preloadedData = {};
      -# do NOT change the following two lines as we're replacing them via a regex in the insert_json_into_dom grunt task
      BB.preloadedData.about = [];
      BB.preloadedData.projects = [];

relevant output with 0.6.1 and 0.8.0 (lacking "about" and "projects"):

    <script type='text/javascript'>BB.preloadedData = {};
    </script>

relevant output with 0.6.0:

<script type='text/javascript'>BB.preloadedData = {};
      BB.preloadedData.about = [];
      BB.preloadedData.projects = [];
</script>

The comment is NOT supposed to appear in the rendered HTML. Am I doing something wrong and is this intended behaviour inside <script> tags?

@joneshf
Copy link
Contributor

joneshf commented Oct 16, 2013

Well, the only thing that happened from 0.6.0 to 0.6.1 was updating haml-coffee from 1.10.x to 1.11.x. So my first guess would be to look there.

As a cursory look, I'd guess it might be related to netzpirat/haml-coffee#67. In particular, this line.

Of course, this is all just a guess, and would require actual diag to figure out the issue.

@paulkoegel
Copy link
Author

thanks for your reply. summoning @netzpirat ;)

@netzpirat
Copy link

I can confirm the problem. Until fixed, you can switch to a filter for the javascript:

%body
  :javascript
    BB.preloadedData = {};
    // do NOT change the following two lines as we're replacing them via a regex in the insert_json_into_dom grunt task
    BB.preloadedData.about = [];
    BB.preloadedData.projects = [];

which works just fine. I'll keep you updated...

@netzpirat
Copy link

Haml-Coffee 1.13.1 released which fixes the issue. Thanks for the failing spec :)

@joneshf
Copy link
Contributor

joneshf commented Oct 17, 2013

@netzpirat Thanks for the timely fix!

@paulwittmann Can you check this and ensure it works for you now?

@paulkoegel
Copy link
Author

hm, still getting:

<script type='text/javascript'>BB.preloadedData = {};
    </script>

grunt-haml 0.8.0 and haml-coffee 1.13.1 now.

$ npm list
├─┬ grunt-haml@0.8.0
│ ├── haml@0.4.3
│ └─┬ haml-coffee@1.13.1

@netzpirat
Copy link

Oops, my spec omitted the {type: 'text/javascript'} (HTML5 FTW), so I didn't catch another issue where the attribute lookahead swallows the next line. This is fixed now in version 1.13.2

@joneshf
Copy link
Contributor

joneshf commented Oct 17, 2013

@paulwittmann What's the word?

@paulkoegel
Copy link
Author

Works now. Thanks a lot! (haml-coffee 1.13.3, grunt-haml 0.8.0)

@paulkoegel
Copy link
Author

Sorry, now there's another issue:

%script{type: 'text/javascript'}
  -# our app's namespace
  window.BB = {};
  BB.env = 'development';
  -# live reload
  document.write('<script src=\"http://' + (location.host || 'localhost').split(':')[0] + ':35729/livereload.js?snipver=1\"><\\/script>');

  WebFontConfig = {
    google: { families: [ 'Roboto:400,100,500,500italic,300italic,300,700:latin', 'Roboto+Condensed:400,700:latin' ] }
  };
  (function() {
    var wf = document.createElement('script');
    wf.src = ('https:' == document.location.protocol ? 'https' : 'http') +
      '://ajax.googleapis.com/ajax/libs/webfont/1/webfont.js';
    wf.type = 'text/javascript';
    wf.async = 'true';
    var s = document.getElementsByTagName('script')[0];
    s.parentNode.insertBefore(wf, s);
  })();

is converted to:

 <script type='text/javascript'>
      WebFontConfig = {
        google: { families: [ 'Roboto:400,100,500,500italic,300italic,300,700:latin', 'Roboto+Condensed:400,700:latin' ] }
      };
      (function() {
        var wf = document.createElement('script');
        wf.src = ('https:' == document.location.protocol ? 'https' : 'http') +
          '://ajax.googleapis.com/ajax/libs/webfont/1/webfont.js';
        wf.type = 'text/javascript';
        wf.async = 'true';
        var s = document.getElementsByTagName('script')[0];
        s.parentNode.insertBefore(wf, s);
      })();
    </script>

works when replacing <script> with :javascript (where I have to use // and which includes my comments in the compilation) except then I get a plain <script> tag in the HTML without a type attribute.

@joneshf
Copy link
Contributor

joneshf commented Oct 23, 2013

I'm not able to test this at the moment, so, can you try with a slightly smaller example? Like:

%script{type: 'text/javascript'}
  var x = 0
  -# Some comment line
  var y = 1

Maybe we can get to the bottom of this.

@joneshf
Copy link
Contributor

joneshf commented Dec 18, 2013

I'm going to assume this is good.

@joneshf joneshf closed this as completed Dec 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants