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

parse and render is not idempotent for bootstrap.min.css #6

Closed
jgm opened this issue Jun 12, 2015 · 9 comments
Closed

parse and render is not idempotent for bootstrap.min.css #6

jgm opened this issue Jun 12, 2015 · 9 comments

Comments

@jgm
Copy link
Contributor

jgm commented Jun 12, 2015

First

wget https://maxcdn.bootstrapcdn.com/bootstrap/3.3.4/css/bootstrap.min.css

Then in GHCI:

GHCI> Data.Text.Lazy.IO.writeFile "converted.css" . Data.Text.Lazy.Builder.toLazyText . Text.CSS.Render.renderNestedBlocks . (\(Right x) -> x) . Text.CSS.Parse.parseNestedBlocks =<< Data.Text.IO.readFile "bootstrap.min.css"

Now observe that bootstrap.min.css is 113K, and converted.css is 14K. Looks like a parsing problem. See jgm/pandoc#2224.

@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

I normalized bootstrap.min.css and converted.css so I could do a diff. The result shows that converted.css is missing everything after this point:

@@ -1510,5535 +1508,3 @@
     font-weight: 300;
     line-height: 1.4;
 }
-
-@media (min-width:768px) {
-    .lead {
-        font-size: 21px;
-    };
-}
-
-.small,small {
-    font-size: 85%;
-}
-
-.mark,mark {
-    padding: .2em;
-    background-color: #fcf8e3;
-}

So, essentially, the parser seems to quit (but without reporting an error) when it hits @media (min-width:768px).

jgm added a commit to jgm/css-text that referenced this issue Jun 28, 2015
Previously arbitrary would only generate a LeafBlock for NestedBlock.
Hence documents with NestedBlock were not really being tested at all!
With this change we can see the bug reported separately in yesodweb#6.
@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

Okay, I think I've localized this bug:

*Text.CSS.Parse> Text.CSS.Parse.parseNestedBlocks "@media print { :after { color: #000!important; } }"
Right [LeafBlock ("@media print",[("","after { color: #000!important")])]

It's the :after selector that is causing the problem.

@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

Culprit is probably line 92 in Parse.hs:

    unknown <- strip <$> takeTill (\c -> c == '{' || c == '}' || c == ':')

@creichert
Copy link
Member

I'm catching up with the issues you linked but there does seem to be an issue:

Prelude Text.CSS.Parse> parseNestedBlocks "@media print { :after { color: #000!important; } }"
Right [LeafBlock ("@media print",[("","after { color: #000!important")])]
Prelude Text.CSS.Parse> parseNestedBlocks "@media print { *:after { color: #000!important; } }"
Right [LeafBlock ("@media print",[("*","after { color: #000!important")])]

With or without the * causes the same problem.

@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

I think I have a fix - will push something soon.

@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

PR #8 should fix the problem discussed most recently above.
However, the bootstrap.min.css test still fails.
The problem now is

@font-face {
    font-family:'Glyphicons Halflings';src:url(../fonts/glyphicons-halflings-regular.eot);src:url(../fonts/glyphicons-halflings-regular.eot?#iefix) format('embedded-opentype'),url(../fonts/glyphicons-halflings-regular.woff2) format('woff2'),url(../fonts/glyphicons-halflings-regular.woff) format('woff'),url(../fonts/glyphicons-halflings-regular.ttf) format('truetype'),url(../fonts/glyphicons-halflings-regular.svg#glyphicons_halflingsregular) format('svg');
}

which chokes the parser.

@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

OK, my mistake. The @font-face problem was introduced by my last change. I had thought that things like

@media {
  property: value; 
}

were invalid CSS, but apparently that's wrong. Will fix.

jgm added a commit to jgm/css-text that referenced this issue Jun 28, 2015
@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

This issue is no longer urgent for me, as I've decided not to use css-text in pandoc.
You may find some useful ideas for improvements in PR #8.

@snoyberg
Copy link
Member

Closing in favor of #8

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