Leftover CSS rules are wrongly escaped

nikolagava commented 1 year ago

Leftover rules for selectors which cannot be applied to elements (e.g. :hover) are escaped in resulting style.

Take the simplest HTML input file in.html

<html>

<head>
    <style>
        .test {
            background-color: red;
            padding: 10px;
        }

        .test:hover {
            background-color: green;
        }
    </style>
</head>

<body>
    <div class="test">Test</div>
</body>

</html>

package main

import (
    "os"

    "github.com/vanng822/go-premailer/premailer"
)

func main() {
    p, err := premailer.NewPremailerFromFile("in.html", premailer.NewOptions())
    if err != nil {
        panic(err)
    }

    out, err := p.Transform()
    if err != nil {
        panic(err)
    }

    f, err := os.Create("out.html")
    if err != nil {
        panic(err)
    }
    defer f.Close()

    f.WriteString(out)
}

After running the Transform(), leftover rules are escaped, which makes it invalid CSS, that is, rules won't be applied to elements.

out.html (formatted for clarity)

<html>

<head>
    <style type="text/css">
        ".test:hover" {
            background-color: green !important
        }
    </style>
</head>

<body>
    <div class="test" style="background-color:red;padding:10px">Test</div>
</body>

</html>

nikolagava commented 1 year ago

After some digging I found the culprit.

https://github.com/vanng822/go-premailer/blob/07efe00b5533d2f445e0be087c334966afe20e7a/premailer/util.go#L7-L16

Line 13 copiedStyle := css.CSSStyleRule{Selector: css.NewCSSValueString(selector), Styles: styles} uses css.NewCSSValueString from your other project https://github.com/vanng822/css which escapes the CSS rule.

func NewCSSValueString(data string) *CSSValue {
    data = strings.ReplaceAll(data, `\`, `\\`)
    data = strings.ReplaceAll(data, `"`, `\"`)
    data = `"` + data + `"`
    token := scanner.Token{scanner.TokenString, data, 0, 0}
    return &CSSValue{Tokens: []*scanner.Token{&token}}
}

Does this have to be changed in css project or is it possible to patch it here?

vanng822 commented 1 year ago

thanks @nikolagava

Please change in css-project. I wonder if it does just appear or it have been there all the time?

nikolagava commented 1 year ago

@vanng822 Looks like these changes were made about 2 years ago. Premailer: https://github.com/vanng822/go-premailer/commit/5392cc8aa5a3c8e7c7168bf2f900db71e1983717 CSS: https://github.com/vanng822/css/commit/6ba7c46733cdfa7f3462d2c4b4d550459eac0a4c

nikolagava commented 1 year ago

But, what if we used css.NewCSSValue instead of css.NewCSSValueString in copyRule function? I see that characters \ and " are escaped. Nevertheless, all premailer tests pass with this one change.

vanng822 commented 1 year ago

Thanks, quite long time not working with those :-D

ok, then probably better to fix it in this repo by using your suggestion. I think it would be ok not escaping since we are not allowed remote loading of css yet.

vanng822 / go-premailer

Leftover CSS rules are wrongly escaped #17