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

Headless printing #36

Open
MartinPaulEve opened this issue Jun 18, 2016 · 8 comments
Open

Headless printing #36

MartinPaulEve opened this issue Jun 18, 2016 · 8 comments

Comments

@MartinPaulEve
Copy link

Hi,

First off, thanks so much for all the work on this implementation. It's really great.

I've been trying, though, to print the output of a page created with this JS using various headless browsers (phantomjs, wkhtml2pdf etc.) and am having no luck. Where, in Chrome within X on Linux (for example), everything looks good (and I can print to PDF), running from the console yields the regions, but with no content. Any ideas?

(I haven't yet tried this with the latest fixes if this was addressed in the last 6 months or so, so please do let me know...)

Best wishes,

Martin

@FremyCompany
Copy link
Owner

Out of the blue, I have no idea. Have you tried waiting some more after the page loaded for the polyfill to do its work?

@MartinPaulEve
Copy link
Author

OK, thanks for the reply.

I did some tracking down here and found what seems to be the problem. Around line 2380 there's a call that is repeated to match the selector "" or ", ". These are casing a massive slowdown in a headless browser. (Taking 10 mins to print a 20 page PDF.)

Changing this block caused me no negative effects and the following worked well for me:

// look if the selector matches
                                var isMatching = false;
                                if(selector != "*" && selector.indexOf("*,") < 0) {
                                    try {
                                        if(element.matches) isMatching=element.matches(selector)
                                        else if(element.matchesSelector) isMatching=element.matchesSelector(selector)
                                        else if(element.oMatchesSelector) isMatching=element.oMatchesSelector(selector)
                                        else if(element.msMatchesSelector) isMatching=element.msMatchesSelector(selector)
                                        else if(element.mozMatchesSelector) isMatching=element.mozMatchesSelector(selector)
                                        else if(element.webkitMatchesSelector) isMatching=element.webkitMatchesSelector(selector)
                                        else { throw new Error("no element.matches?") }
                                    } catch(ex) { debugger; setImmediate(function() { throw ex; }) }

                                    // if yes, add it to the list of matched selectors
                                    if(isMatching) { results.push(subrules[sr]); }
                                }

I don't know whether this might be useful to merge upstream? If so, I can put in a PR...

Best wishes,

Martin

@FremyCompany
Copy link
Owner

Strange. I bet the slowness comes from an invalid selector, which forces the browser to enter the catch section, and might be the root cause of the problem.

Can you please print a callstack and the value of "rule", "rule.selector" and "selector" of cases where the selector "" and "," is being looked for? It seems likely it is due to some (set of) css rules in particular, and it would be interesting to understand which.

@MartinPaulEve
Copy link
Author

Thanks for this. Can I just check: did your post (in the selectors) get messed up by formatting? (Your "" and "," appears in italics and I want to check markdown hasn't killed it before I investigate).

As above, this happens when selector is "*". Happy to investigate further, though, if you can clarify exactly what selectors I should be looking for.

@FremyCompany
Copy link
Owner

Ahaha, yes, I meant * and *,…. Markdown ate both our * selectors, your post has the same issue :)

@MartinPaulEve
Copy link
Author

OK, so this happens hundreds of times per run, but I get the following for a print of rule, rule.selector, and then a console.trace done in Firefox rather than a command-line tool (this is just one example. The stacktrace is different in different cases. Also, the line numbers are slightly out in some cases because I've modified the script to debug it):

2016-06-21T08:01:12.816Z [INFO    ] Ghost<44c1d8b8-2cba-44b9-a6f9-487087d76051>: http://localhost:8000/regions/css-regions-polyfill.js(2401): {"prelude":[{"token":"DELIM"},{"token":"WHITESPACE"}],"selector":[{"token":"DELIM"},{"token":"WHITESPACE"}],"value":{"name":"{","value":[{"token":"WHITESPACE"},{"token":"IDENT"},{"token":":"},{"token":"WHITESPACE"},{"token":"NUMBER","value":0,"type":"integer","repr":"0"},{"token":";"},{"token":"WHITESPACE"},{"token":"IDENT"},{"token":":"},{"token":"WHITESPACE"},{"token":"NUMBER","value":0,"type":"integer","repr":"0"},{"token":";"},{"token":"WHITESPACE"}]},"subRules":[{"prelude":[{"token":"DELIM"},{"token":"WHITESPACE"}],"selector":[{"token":"DELIM"},{"token":"WHITESPACE"}],"value":{"name":"{","value":[{"token":"WHITESPACE"},{"token":"IDENT"},{"token":":"},{"token":"WHITESPACE"},{"token":"NUMBER","value":0,"type":"integer","repr":"0"},{"token":";"},{"token":"WHITESPACE"},{"token":"IDENT"},{"token":":"},{"token":"WHITESPACE"},{"token":"NUMBER","value":0,"type":"integer","repr":"0"},{"token":";"},{"token":"WHITESPACE"}]}}]}
2016-06-21T08:01:12.816Z [INFO    ] Ghost<44c1d8b8-2cba-44b9-a6f9-487087d76051>: http://localhost:8000/regions/css-regions-polyfill.js(2402): DELIM(*),WS


findAllMatchingRules/visit() css-regions-polyfill.js:2401
findAllMatchingRules() css-regions-polyfill.js:2421
visit/importPseudo() css-regions-polyfill.js:4530
visit() css-regions-polyfill.js:4564
window.cssRegionsHelpers.copyStyle() css-regions-polyfill.js:4592
module.exports/cssRegions.Flow.prototype.generateContentFragment() css-regions-polyfill.js:4840
module.exports/cssRegions.Flow.prototype._relayout() css-regions-polyfill.js:4967
module.exports/cssRegions.Flow.prototype.relayout/<() css-regions-polyfill.js:4896
visit() css-regions-polyfill.js:4473
visit() css-regions-polyfill.js:4582
visit() css-regions-polyfill.js:4582
visit() css-regions-polyfill.js:4582
window.cssRegionsHelpers.copyStyle() css-regions-polyfill.js:4592
module.exports/cssRegions.Flow.prototype.generateContentFragment() css-regions-polyfill.js:4840
module.exports/cssRegions.Flow.prototype._relayout() css-regions-polyfill.js:4967
module.exports/cssRegions.Flow.prototype._relayout/<.onprogress() css-regions-polyfill.js:4989
cssRegions.layoutContentInNextRegionsWhenReady() css-regions-polyfill.js:5476
cssRegions.layoutContent() css-regions-polyfill.js:5393
cssRegions.layoutContentInNextRegionsWhenReady() css-regions-polyfill.js:5472
cssRegions.layoutContent() css-regions-polyfill.js:5393
cssRegions.layoutContentInNextRegionsWhenReady() css-regions-polyfill.js:5472
cssRegions.layoutContent() css-regions-polyfill.js:5393
module.exports/cssRegions.Flow.prototype._relayout() css-regions-polyfill.js:4976
module.exports/cssRegions.Flow.prototype.relayout/<()

@FremyCompany
Copy link
Owner

FremyCompany commented Jun 21, 2016

Oh, I see, that happens during copyStyle which manually computes the cascaded style of elements. This is not the polyfill framework trying to see which elements match the polyfilled css region properties.

The funny part about it is that technically this callstack lies within an optimization.
https://github.com/FremyCompany/css-regions-polyfill/blob/master/bin/css-regions-polyfill.js#L4523

When cloning a fragment, I try to see which pseudo-elements exist and what their style might be, to preserve them in the cloned segments. For some reason this is very slow in your case, probably because you have some *:before, *:after { ... } selector in your stylesheet, which tricks the engine to believe every element might have a ::before and ::after pseudo-element, and then of course this triggers a pretty heavy process of trying to access pseudo-elements on every dom node the page, which used to create them if they did not exist in older versions of webkit/blink, which triggers a lot of slow operations under the hood.

Your fix ignores those misleading selectors which likely improves the performance very visibly. The right fix would be to change the "mayExist" condition not to be the existence of a rule matching the pseudo, but the existence of such are rule that contains a content: ... declaration; all the other ones should be ignored.
https://github.com/FremyCompany/css-regions-polyfill/blob/master/bin/css-regions-polyfill.js#L452

@MartinPaulEve
Copy link
Author

Ah ha. Very interesting. Thanks so much for explaining.

I have this at the top of my stylesheet:

*, *:before, *:after {
  -moz-box-sizing: border-box; -webkit-box-sizing: border-box; box-sizing: border-box;
}

and can now confirm that this is causing a major slowdown. Removing this doesn't affect my layout, in fact, so I can safely get rid of them!

Thanks for investigating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants