donpark / html2jade

Converts HTML to Jade template. Not perfect but useful enough for non-daily conversions.
MIT License
1.18k stars 156 forks source link

Doesn't like [if lt IE 7] style conditionals. #60

Closed MichaelJCole closed 10 years ago

MichaelJCole commented 11 years ago

Hi, I just converted this HTML and it munged the conditionals that set the html tag. This HTML Is from http://www.initializr.com/:

HTML

<!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>
        <meta charset="utf-8">
        <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
        <title></title>
        <meta name="description" content="">
        <meta name="viewport" content="width=device-width">

        <link rel="stylesheet" href="css/bootstrap.min.css">
        <style>
            body {
                padding-top: 50px;
                padding-bottom: 20px;
            }
        </style>
        <link rel="stylesheet" href="css/bootstrap-theme.min.css">
        <link rel="stylesheet" href="css/main.css">

        <script src="js/vendor/modernizr-2.6.2-respond-1.1.0.min.js"></script>
    </head>
    <body>
        <!--[if lt IE 7]>
            <p class="chromeframe">You are using an <strong>outdated</strong> browser. Please <a href="http://browsehappy.com/">upgrade your browser</a> or <a href="http://www.google.com/chromeframe/?redirect=true">activate Google Chrome Frame</a> to improve your experience.</p>
        <![endif]-->
    <div class="navbar navbar-inverse navbar-fixed-top">
</div>
</body>

Result (notice the missing conditionals)

!!! 5
html.no-js
//
    <![endif]
  head
    meta(charset='utf-8')
    meta(http-equiv='X-UA-Compatible', content='IE=edge,chrome=1')
    title
    meta(name='description', content='')
    meta(name='viewport', content='width=device-width')
    link(rel='stylesheet', href='css/bootstrap.min.css')
    style
      body {
      padding-top: 50px;
      padding-bottom: 20px;
      }
    link(rel='stylesheet', href='css/bootstrap-theme.min.css')
    link(rel='stylesheet', href='css/main.css')
    script(src='js/vendor/modernizr-2.6.2-respond-1.1.0.min.js')
  body
    //if lt IE 7
      p.chromeframe
        | You are using an
        strong outdated
        | browser. Please
        a(href='http://browsehappy.com/') upgrade your browser
        | or
        a(href='http://www.google.com/chromeframe/?redirect=true') activate Google Chrome Frame
        | to improve your experience.
    .navbar.navbar-inverse.navbar-fixed-top
donpark commented 11 years ago

yup, caught "no-js" but not "lt-ie9 lt-ie8 lt-ie7". hmm

donpark commented 11 years ago

weird. conditional2.html test case is very similar to yours yet extra conditionals are not dropped in its output.

donpark commented 11 years ago

even more weird, latest version now converts both [conditional2.html] and your HTML to nothing. Since html2jade hasn't changed much, dependent module change that broke this, likely jsdom.

donpark commented 11 years ago

Unfortunately, I don't have time to dig further into this right now.

If you are in a hurry, try installing older versions of jsdom.

MichaelJCole commented 11 years ago

Hey, thanks for looking into it! I've used your site for alot of converting and it's been extrememly helpful :-)

donpark commented 11 years ago

you're welcome. I'm expected to be stuck in crunch mode until end of November but I'll try to take opportunity shots meanwhile. thanks for your patience. :-)

donpark commented 11 years ago

oh heck. I went ahead and tried different versions of jsdom and found latest working version of jsdom is 0.6.5 so released a version locked to that version of jsdom. hope that'll let you do what you need done.

Meanwhile, I'll leave this issue open to figure out what changes in jsdom broke this. It's not htmlparser2. I've checked.

donpark commented 10 years ago

As of version 0.7, jsdom-little is used which apparently don't have this issue. Closing for now. Please reopen if issue reoccurs.