Upgrade to 7.0.0 breaks CSS?

[jasonb-vbt]

Has anyone else seen this? I upgraded to the new markdown-pdf 7.0.0 in order to get the new links-in-PDFs behavior, only to find that now my CSS is broken. As in, either markdown-pdf or PhantomJS (can't tell which) just isn't looking at my CSS file at all anymore. The output is exactly the same as if I take the cssPath value out of my markdown-pdf options entirely.

I have changed literally nothing in my app. All I did was npm update markdown-pdf, and wham, CSS is busted. I've double-checked that the CSS file really is still there, et cetera. Here's how I'm invoking markdown-PDF:

fileList = buildFileList() // collect file information from src/documents/*.md
var options = {
    preProcessMd: preProcessMd,
    cssPath: "src/static/styles/default.css",
    remarkable: {
        html: true
    }
}

markdownpdf(options)
    .concat.from(
        fileList.map(function(val){ return val.path } )
    )
    .to(getOutputFilename(fileList[0].title), function () {
  console.log("Done.")
})

[anko]

I can't reproduce this. I created a new unit test for this in https://github.com/anko/markdown-pdf/commit/3008eeb6cb48454f559a287ce2f122ec6eb2b78a which passes in CI.

Some ideas for next steps:

• What OS are you on? (To rule out different PhantomJS executables.)
• Could you test what happens when using only the cssPath option? (To rule out any interaction between different options.)

An MCVE would be super helpful.

[jasonb-vbt]

Ok. I had some time to look at this today. I'm running this on Windows 7. The issue repros when using just the cssPath option. I think the following counts as an MCVE. Put all files in the same test directory an run node buildpdf.js :

buildpdf.js:

var markdownpdf = require("markdown-pdf")

var optionsCSS = {
    cssPath: "./default.css",
}

var optionsNoCSS = {
}

markdownpdf(optionsCSS)
    .concat.from(["my.html.md"])
    .to("withCSS.pdf", function () {
  console.log("Made CSS version.")
})

markdownpdf(optionsNoCSS)
    .concat.from(["my.html.md"])
    .to("NoCSS.pdf", function () {
  console.log("Made NoCSS version.")
})

default.css:

h1 {
  color: #FF0000;
  font-size: 44px;
  line-height: 56px;
  font-weight: 300;
}

my.html.md:

# 1 This Heading Should Be Big And Red

This is just a paragraph

[jasonb-vbt]

Addendum: This morning I tried uninstalling and re-installing all node packages on my system, just in case. The missing CSS problem persists.

[anko]

@jasonb-vbt Thanks for the thorough test. We clearly have a broken build.

On Linux, your inputs give me the expected output documents:

Markdown-pdf's changes from the previous version (v6.0.0) to to the current (v7.0.0) are pretty trivial and I see nothing in them that could break CSS in a platform-specific way—the only notable functional change is the new PhantomJS.

Just to make sure, let's try taking markdown-pdf out of the equation. Try running this gist on Windows. It uses phantomjs-prebuilt directly on an HTML file to render a PDF. That should tell us some more.

I'll try to get a Windows machine to a state where I could repro this myself, but no promises.

---

This could have been avoided if we had a CI server that tested Windows. Our current Travis CI can't do Windows. @alanshaw What are your thoughts on this—e.g. unit testing on Windows via AppVeyor? I tried copying this AppVeyor config for Node into this branch on my fork which died on something silly.

[mattbrooks2010]

:+1:

[jasonb-vbt]

@anko Your gist works as expected, producing a big red h1.

The tactic you took of using a style block directly in the html is a strategy I've been playing with the weekend, with rather confusing results. But the more I kick the tires on this thing, the more I agree that it's a problem with the new phantomjs. What I know so far is that if I hack my preProcessMd function to insert a style block that just contains some simple css as in your gist, it works. But if I have it dump in the entire contents of my css file (which, remember, worked just fine before 7.0.0), then all the styles mysteriously vanish.

I am suspicious that maybe it's a bug in phantomjs's CSS processing, that maybe it's choking on some particular bit of styling I happen to be using, so I've been putting pieces of my CSS in and out in an attempt to figure out exactly which bit is breaking it, but I don't have a definitive result so far.

[jasonb-vbt]

FYI, this is no longer a blocking issue for me. I switched from markdown-pdf to marked for md-to-html conversion plus wkhtmltopdf for html to pdf. Works great.

I still think the underlying issue for me was with phantom, not with markdown-pdf itself. What finally made me give up on this particular toolchain was converting all my .md files to one monolithic HTML, with a

Thanks for all your efforts in helping me narrow this down. If you want to close this issue as no-repro, I won't object.

[animeoakes]

Adding cssPath = 'file:///' + cssPath to the page.evaluate block of render.js fixes the problem for me.

// Add custom CSS to the page
page.evaluate(function (cssPaths) {
  var head = document.querySelector('head')
  cssPaths.forEach(function (cssPath) {
    cssPath = 'file:///' + cssPath
    var css = document.createElement('link')
    css.rel = 'stylesheet'
    css.href = cssPath

    head.appendChild(css)
  })
}, [args.cssPath, args.highlightCssPath])

[alanshaw]

Hi @bmahsman will you send a PR?

[Jonlondon]

Thanks @bmahsman it does the job. Should send PR !

[roadmanfong]

Hi, I'm developing in Windows and have the same problem here. Do you have any publish plan recently? Thank you.

[animeoakes]

I am trying to prepare a PR, however I am having trouble detecting the OS to be Windows to conditionally append file:/// to the CSS paths. The expression os.name === 'windows' is not returning true within the evaluate callback. I'm not familiar with PhantomJS, so I don't know how to console.log(os.name) and see what it's actually telling us the OS is within that evaluate callback. Any ideas?

[roadmanfong]

https://nodejs.org/api/os.html#os_os_platform

Did you try os.platform() === 'win32' ?

[RogerKang]

@bmahsman Great! Your suggestion work for me on windows!!Thank you!

[r0]

I tried os.paltform() === 'win32' but it didn't work. I'm also not familiar with Phantom.JS and It would be really appreciated if someone could help.

[lokeshjain20]

@bmahsman thank you so much for your suggestion. How did you found it?

[alanshaw]

@bmahsman in phantomjs you'd use the "system" module - http://phantomjs.org/api/system/property/os.html

[purtuga]

Thank you @bmahsman - your suggestio nworks for me (Window 8, PhantomJS Prebuilt v (from package.json): 2.1.14).