Not all "critical" CSS being included (Bootstrap v4 alpha 5) #181

kenshaw · 2016-12-03T07:29:41Z

My apologies in advance if this is not a critical issue, if I am posting this in the wrong place, if this is due to my misunderstanding of how to use critical, or if this is is instead a bootstrap problem.

Using the latest version of critical, v0.8.1, and using Bootstrap v4 alpha 2 in this HTML:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <title>Error Encountered During Authorization</title>
  <link href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-alpha.2/css/bootstrap.min.css" rel="stylesheet">
</head>
<body>
  <div class="container-fluid m-t-1">
    <div class="row">
      <div class="col-xs-5">
        <div class="card card-block">
          <h4 class="card-title">Error{{ if .Description }}: {{ .Title }}{{ end }}</h4>
          <h5>An error was encountered processing the authorization request:</h5>
          <p class="card-text">
            {{ if .Description }}{{ .Description }}{{ else }}{{ .Title }}{{ end }}
          </p>
        </div> <!-- end card -->
      </div> <!-- end col-xs-5 -->
    </div> <!-- end row -->
 </div> <!-- end container-fluid -->
</body>
</html>

And running critical like the following:

critical ae_bs4_a2.html --inline > ae_bs4_a2.out.html

Will correctly include the '.card' CSS selector. However, with Bootstrap v4 alpha 5 and this html (note, the only changes are the CDN location, and the m-t-1 class name was changed to mt-1):

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <title>Error Encountered During Authorization</title>
  <link href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-alpha.5/css/bootstrap.min.css" rel="stylesheet">
</head>
<body>
  <div class="container-fluid mt-1">
    <div class="row">
      <div class="col-xs-5">
        <div class="card card-block">
          <h4 class="card-title">Error{{ if .Description }}: {{ .Title }}{{ end }}</h4>
          <h5>An error was encountered processing the authorization request:</h5>
          <p class="card-text">
            {{ if .Description }}{{ .Description }}{{ else }}{{ .Title }}{{ end }}
          </p>
        </div> <!-- end card -->
      </div> <!-- end col-xs-5 -->
    </div> <!-- end row -->
 </div> <!-- end container-fluid -->
</body>
</html>

And running critical like the following:

critical ae_bs4_a5.html --inline > ae_bs4_a5.out.html

Will produce inline'd css that DOES NOT include the .card classes (and others).

Is this a critical issue, or am I doing something incorrect here?

The text was updated successfully, but these errors were encountered:

kenshaw · 2016-12-03T07:34:20Z

Sorry, forgot to mention that this causes a "flash of unstyled content" on the v5 version prior to the maxcdn css being retrieved.

kenshaw · 2016-12-03T07:40:02Z

Not sure if this is helpful or not, but I also just checked the bootstrap v4 alpha 3 and alpha 4, and they include the correct classes.

pocketjoso · 2016-12-03T10:21:59Z

@kenshaw It would be a Penthouse problem; Penthouse is what generates the critical css. I'll give it a try.

pocketjoso · 2016-12-03T10:26:05Z

.. although I notice that what you've posted is not html. Isn't that causing problems? I'm not completely up to date to if critical does any preprocessing to transpile certain template languages to html.

pocketjoso · 2016-12-03T10:45:33Z

I can confirm that something causes the .card rule(s) to go missing in the critical css given by Penthouse. Looking into why that's the case.

pocketjoso · 2016-12-03T11:20:43Z

So it's an issue with the minified bootstrap css and the AST parser Penthouse uses (this one, you can see the problem here:

http://iamdustan.com/reworkcss_ast_explorer/#/e8Tdpm7KWq/1

The .card rule, and a lot of other rules, are being swallowed by a base64 background-image url declaration value.

@kenshaw : Hmm. If you were using Penthouse directly you could fix this problem for yourself by unminifying the full css before calling Penthouse... However using critical you don't really have this option... yet.

@bezoerb and other critical maintainers: I have found no better fix in Penthouse for this type of problem than unminifying the full css, however I don't want to bake unminification into the Penthouse library. I want to keep the scope of Penthouse small, and Penthouse was meant to be used in a build step, on pre-production css, so this problem should be less common for pure Penthouse users.

Since critical works directly with html files however, it seems more likely that users will show up with minified css assets. Could you start unminifying the css before calling Penthouse (unminifyCss: true)? This type of AST parsing problem is not very common, but there are some valid base64 declaration values that cause problems. I am also looking in parallel for a more forgiving AST parser, in case you know one.

kenshaw · 2016-12-03T12:12:46Z

Thanks for the quick response. I can confirm that using the non-minified version of Bootstrap makes this issue go away.

Really confusing though, but I really appreciate you guys working on this! Many thanks!

lukebussey · 2016-12-13T03:35:08Z

I'm running into the same issue, but because of my particular build process, I can't unminify the CSS beforehand.

The parsing issue is caused by this line in Bootstrap's _variables.scss file:

$navbar-light-toggler-bg: url("data:image/svg+xml;charset=utf8,%3Csvg viewBox='0 0 32 32' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath stroke='#{$navbar-light-color}' stroke-width='2' stroke-linecap='round' stroke-miterlimit='10' d='M4 8h24M4 16h24M4 24h24'/%3E%3C/svg%3E");

The part causing the problem is the stroke color: stroke='#{$navbar-light-color}' which is compiled to: stroke='rgba(0,0,0,.5)'

Changing this to a solid hex color E.g. stroke='#7F7F7F' fixes the issue.

midzer · 2017-10-07T08:30:53Z

Great find @lukebussey
Lets see if i can dig more into this issue.

midzer · 2017-12-16T14:19:07Z

Tbh i have no clue where to start on this issue :[

Furthermore with quick fix from @lukebussey it appears latest v1+ critical extracts only navbar components for me. This results in too less CSS being inlined for my project. Too bad I had to revert to v0.8.4.

pocketjoso · 2017-12-16T14:33:40Z

This (incorrect parsing of original css, especially when containing errors) has been resolved in the latest (1.2) version of Penthouse - https://github.com/pocketjoso/penthouse/releases/tag/v1.2.0.

@bezoerb @addyosmani Would be great to see an upgrade pushed out.

midzer · 2018-01-08T13:27:10Z

@lukebussey It appears latest penthouse doesn't need this bootstrap hack any more.

Miserably, there is still some CSS missing for my critical gulp task.
I tried the online version via https://jonassebastianohlsson.com/criticalpathcssgenerator/ and all necessary CSS is extracted.

@pocketjoso I have no idea where to look. Do you have any idea what is wrong with my critical task config? Thanks in advance! https://github.com/midzer/eisolzried/blob/master/_tasks/critical.js

pocketjoso · 2018-01-08T18:34:48Z

@midzer The online generator is on the latest penthouse version - critical is not. I presume this is the culprit. With the latest penthouse version no more hacks are needed to get around CSS parsing issues.

Friendly ping @bezoerb for a release update - did you try the latest Penthouse version (1.3, although the majority of Penthouse became fault tolerant in 1,2)? If you encountered any issues, please let me know. If there was just no time available to test it yet then I fully understand!

midzer · 2018-01-08T18:41:58Z

@pocketjoso in my build environment package.json of penthouse in node_modules folder states I'm currently using version 1.3.0.

bezoerb · 2018-01-08T22:24:13Z

Thanks for the ping @pocketjoso :)
I already worked on the dependency update in the bump-dependencies branch.
Unfortunately i'm randomly running in some ECONNRESET errors and i haven't yet figured out what's causing them.

pocketjoso · 2018-01-10T21:15:23Z

Thanks for the update!

…

On Mon, Jan 8, 2018 at 11:24 PM, Ben Zörb ***@***.***> wrote: Thanks for the ping @pocketjoso <https://github.com/pocketjoso> :) I already worked on the dependency update in the bump-dependencies <https://github.com/addyosmani/critical/tree/bump-dependencies> branch. Unfortunately i'm randomly running in some ECONNRESET errors <https://travis-ci.org/addyosmani/critical/builds/319980576> and i haven't yet figured out what's causing them. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#181 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG3BANdB41k7QF_Yy1x1_PnBuQDk4pTBks5tIpWPgaJpZM4LDOg_> .

-- https://jonassebastianohlsson.com <http://jonassebastianohlsson.com> @pocketjoso <http://twitter.com/pocketjoso>

midzer · 2018-03-31T13:35:49Z

Bumping this one as css generated from latest critical still differs from css generated from https://jonassebastianohlsson.com/criticalpathcssgenerator/

My critical config:

inline: true,
base: '_site/',
css: ['_site/assets/css/main.min.css'],
minify: true

Site: https://feuerwehr-eisolzried.de/
Having a quick look what is missing: all carousel class, all necessary col-* classes, row class.

pocketjoso · 2018-03-31T19:04:59Z

@midzer Critical is currently using the latest version pf Penthouse, just as my online generator. Hence if you have a difference it should be related to the input - that critical for some reason is not seeing the original css containing these styles. Are these styles coming from a separate source than the ones that are kept?

midzer · 2018-03-31T19:23:39Z

@pocketjoso Thx for your reply
All styles are located in main.min.css. Specifying this file in css parameter makes no difference either.

pocketjoso · 2018-03-31T19:37:07Z

Hmm but then you really should get the same critical styles from critical as from the online generator. Are you sure these styles are critical - shown in the critical viewing for the first render (before js)? Le sam. 31 mars 2018 21:23, midzer <[email protected]> a écrit :

…

@pocketjoso <https://github.com/pocketjoso> Thx for your reply All styles are located in main.min.css. Specifying this file in css parameter makes no difference either. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#181 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG3BAFV1lnSzWuNZfMr417YGaQtkKOD7ks5tj9e-gaJpZM4LDOg_> .

midzer · 2018-03-31T20:13:35Z

I really don't know whats going on there :\

Here's a diff between online generator css (left) and inlined critical css (right) using before stated settings https://www.diffchecker.com/ysBsWWiz

pocketjoso · 2018-04-01T07:46:39Z

@midzer Tip: the comparison is much easier to understand if your unminify the css first.

So it looks to me that somehow you are feeding the wrong html/url into critical. That would explain why the page it is looking at is not what you think it is, not containing the content you think, and hence why you get a different critical css.

So can you double check your critical setup, and maybe try with a completely different url? I suspect this is the problem on your end.

bezoerb · 2018-04-01T21:04:54Z

@midzer: I completely agree with @pocketjoso
When i run critical from the commandline like this:

critical https://feuerwehr-eisolzried.de

I get the same results without anything missing.
https://www.diffchecker.com/NZzBnVEn

midzer · 2018-04-02T19:56:20Z

First of all thx again for your help @pocketjoso and @bezoerb

I figured out whats going on! Finally using local production build critical css is being fully generated same as via online gerator or critical cli + url.

Right now there's a caveat though. It's only fully generated if I specify

<link rel="stylesheet" href="assets/css/main.css">

in HTML <head> instead of regular

<link rel="stylesheet" href="/assets/css/main.css">

But I need this beginning / as otherwise other sub-pages could't find respective css file any more.

After digging in some related issues I tried to specify "" for pathPrefix but this is of no help at all.
Setting optional css parameter to ['_site/assets/css/main.css'] didn't work, too.

midzer · 2018-05-20T09:27:29Z

Update to previous post:
I still can't inline full critical CSS automatically and have to do it manually which is kinda annoying.

If some1 has time to help me out on this issue, would be really awesome.

bezoerb · 2019-01-08T16:15:31Z

@midzer: sorry for the late response.
I'm working on a new release of critical. You can install it using npm i critical@next

Maybe you could check if this update solves your issue?

MindSculpt · 2019-04-11T18:20:38Z

@midzer I can confirm this is still an issue. I'm able to generate critical css inline perfectly, but only when the path to your main css file is relative:

<link rel="stylesheet" href="assets/css/main.css">

...which makes it really difficult to try to use this for anything other than your home page.

bezoerb · 2019-04-11T18:32:40Z

@MindSculpt: can you provide a gist or something else to reproduce the issue?

MindSculpt · 2019-04-11T19:13:14Z

@bezoerb That might be tough, as critical is built into my custom static website generator which is not part of a public repo.

When I use an absolute path to my stylesheet, critical starts up but then stops suddenly before grabbing the rest of the styles needed, so I wind up with FOUC because the inlined styles aren't complete. I am using Bootstrap 4 in this build (which relates to one of the issues above). I am also running critical from a gulp task that's not in the root of my project, so I'm not sure if that's part of it. I am using critical@next with Bootstrap 4.

In previous projects, critical worked fine with Bootstrap 3, so I don't know if it's a BS4 issue or not.

bezoerb · 2019-06-08T21:14:51Z

I tried to reproduce this behavior but everything works just as expected when i use Bootstrap 4. If anyone can provide something to reproduce this issue leave me a note ;)

XhmikosR · 2019-08-07T06:02:15Z

Works also fine here for my own projects. If this is still an issue please make a new issue with all a minimal reproducible example included (package.json and everything).

bezoerb added the bug label Jan 27, 2017

bezoerb self-assigned this Dec 16, 2017

pocketjoso mentioned this issue Jan 13, 2018

Unhandled exception breaks down grunt task pocketjoso/penthouse#213

Closed

mohitsinghs mentioned this issue Feb 1, 2018

1.*.* breaks gulp task #271

Closed

XhmikosR closed this as completed Aug 7, 2019

kanenas mentioned this issue Jun 21, 2023

[Snyk] Fix for 1 vulnerabilities kanenas/critical#36

Open

shrndlward mentioned this issue Jun 21, 2023

[Snyk] Fix for 1 vulnerabilities shrndlward/critical#15

Open

Subhojit1992 mentioned this issue Jun 21, 2023

[Snyk] Fix for 1 vulnerabilities Subhojit1992/critical#36

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Not all "critical" CSS being included (Bootstrap v4 alpha 5) #181

Not all "critical" CSS being included (Bootstrap v4 alpha 5) #181

kenshaw commented Dec 3, 2016

kenshaw commented Dec 3, 2016

kenshaw commented Dec 3, 2016

pocketjoso commented Dec 3, 2016

pocketjoso commented Dec 3, 2016

pocketjoso commented Dec 3, 2016

pocketjoso commented Dec 3, 2016 •

edited

Loading

kenshaw commented Dec 3, 2016

lukebussey commented Dec 13, 2016

midzer commented Oct 7, 2017

midzer commented Dec 16, 2017

pocketjoso commented Dec 16, 2017 •

edited

Loading

midzer commented Jan 8, 2018

pocketjoso commented Jan 8, 2018

midzer commented Jan 8, 2018

bezoerb commented Jan 8, 2018

pocketjoso commented Jan 10, 2018 via email

midzer commented Mar 31, 2018

pocketjoso commented Mar 31, 2018

midzer commented Mar 31, 2018

pocketjoso commented Mar 31, 2018 via email

midzer commented Mar 31, 2018

pocketjoso commented Apr 1, 2018

bezoerb commented Apr 1, 2018

midzer commented Apr 2, 2018

midzer commented May 20, 2018

bezoerb commented Jan 8, 2019

MindSculpt commented Apr 11, 2019 •

edited

Loading

bezoerb commented Apr 11, 2019

MindSculpt commented Apr 11, 2019

bezoerb commented Jun 8, 2019

XhmikosR commented Aug 7, 2019

Not all "critical" CSS being included (Bootstrap v4 alpha 5) #181

Not all "critical" CSS being included (Bootstrap v4 alpha 5) #181

Comments

kenshaw commented Dec 3, 2016

kenshaw commented Dec 3, 2016

kenshaw commented Dec 3, 2016

pocketjoso commented Dec 3, 2016

pocketjoso commented Dec 3, 2016

pocketjoso commented Dec 3, 2016

pocketjoso commented Dec 3, 2016 • edited Loading

kenshaw commented Dec 3, 2016

lukebussey commented Dec 13, 2016

midzer commented Oct 7, 2017

midzer commented Dec 16, 2017

pocketjoso commented Dec 16, 2017 • edited Loading

midzer commented Jan 8, 2018

pocketjoso commented Jan 8, 2018

midzer commented Jan 8, 2018

bezoerb commented Jan 8, 2018

pocketjoso commented Jan 10, 2018 via email

midzer commented Mar 31, 2018

pocketjoso commented Mar 31, 2018

midzer commented Mar 31, 2018

pocketjoso commented Mar 31, 2018 via email

midzer commented Mar 31, 2018

pocketjoso commented Apr 1, 2018

bezoerb commented Apr 1, 2018

midzer commented Apr 2, 2018

midzer commented May 20, 2018

bezoerb commented Jan 8, 2019

MindSculpt commented Apr 11, 2019 • edited Loading

bezoerb commented Apr 11, 2019

MindSculpt commented Apr 11, 2019

bezoerb commented Jun 8, 2019

XhmikosR commented Aug 7, 2019

pocketjoso commented Dec 3, 2016 •

edited

Loading

pocketjoso commented Dec 16, 2017 •

edited

Loading

MindSculpt commented Apr 11, 2019 •

edited

Loading