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.

Sign up for GitHub

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

Jump to bottom

Comments go missing when replacing script tag #67

Closed
trodrigues opened this issue Apr 11, 2013 · 4 comments
Closed

Comments go missing when replacing script tag #67

trodrigues opened this issue Apr 11, 2013 · 4 comments
Assignees
@asciidisco
Labels
bug

Comments

@trodrigues
Copy link

Following up from the discussion on Twitter, here's a better description of the problem and what I've found out about it.

In this specific case, the HTML file has something like this:

<!DOCTYPE html>
<!--[if lt IE 7]>      <html class="no-js lt-ie9 lt-ie8 lt-ie7"> <![endif]-->
<!--[if IE 7]>         <html class="no-js lt-ie9 lt-ie8"> <![endif]-->
<!--[if IE 8]>         <html class="no-js lt-ie9"> <![endif]-->
<!--[if gt IE 8]><!--> <html class="no-js"> <!--<![endif]-->
  <head>

After passing through grunt-requirejs with the replaceRequireScript option turned on, we end up with the following:

<!DOCTYPE html>
<html class="no-js"><!--<![endif]--><head>

After having a look through the code and doing some debugging, I found what the cause of the issue is, but I'm not really sure of what a best solution would be for this case.

At the end of lib/replace.js, the contents of the modified file which has just been parsed through domino are written into the file:

// write out newly created file contents
grunt.file.write(file, getDoctype(document) + document.documentElement.outerHTML);

The problem is that when the code goes into domino, it's properly being parsed as a DOM tree, and in our original code there are actually 3 different start tags for . Apparently, domino uses the last one as the actual start of the node, and that's what it outputs when we get the outerHTML of the document.documentElement, which in a way, makes complete sense.

The innerHTML of document does have the whole thing, but without newlines around those comments, and it does still contain the doctype:

<!DOCTYPE html><!--[if lt IE 7]>      <html class="no-js lt-ie9 lt-ie8 lt-ie7"> <![endif]--><!--[if IE 7]>         <html class="no-js lt-ie9 lt-ie8"> <![endif]--><!--[if IE 8]>         <html class="no-js lt-ie9"> <![endif]--><!--[if gt IE 8]><!--><html class="no-js"><!--<![endif]--><head>

I don't think it would be desirable to output the HTML like this from the plugin, so I thought it was probably better to have some brain storming around a potential better fix for this.
@ghost ghost assigned asciidisco May 7, 2013
@dalgard
Copy link

dalgard commented Jun 13, 2013

+!

This is a problem for me, too. Need those IE conditional comments. Any work-arounds so far?

@andrey-buligin
Copy link

Did anyone found a solution for this problem?

@nitinhayaran
Copy link
Contributor

+1

@asciidisco
Copy link
Owner

Fixed in 0.4.1. Huge thanks to @nitinhayaran for sending in a PR

@asciidisco asciidisco mentioned this issue Aug 13, 2014
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@asciidisco asciidisco

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@trodrigues @nitinhayaran @asciidisco @dalgard @andrey-buligin