search

maxwell / screencap

A gem to screencap webpages in ruby. Uses Phantom.js under the hood.
MIT License
179 stars 63 forks source link

resourceWait's delay does not have an effect #6

Closed donpinkus closed 9 years ago

donpinkus commented 10 years ago

It seems "resourceWait" isn't actually causing a wait. When I run this, the screenshot will still be taken in ~2-3 seconds.

f = Screencap::Fetcher.new('https://google.com')

screenshot_file = f.fetch(
        output: '~/screenshot.png',
        resourceWait: 10000,
        maxRenderWait: 15000)
donpinkus commented 10 years ago

Hey guys - is this gem still maintained? If so I'll write a pull request, if not I can fork. Thanks!

maxwell commented 10 years ago

We released a new version a couple of months ago. Happy to accept your pull request.

donpinkus commented 10 years ago

Sweet will do - it's a really useful gem.

I'm testing it locally right now, have you guys heard of any other issues with resourceWait / does it work for you?

christophersansone commented 9 years ago

I came across this issue today and believe I got it working. It seems that the root problem is due to maxRenderWait occurring unexpectedly ahead of time. The resourceWait parameter is in fact behaving properly, but maxRenderWait is beating it to the punch.

My understanding of the two parameters is that resourceWait specifies the amount of time to wait until after all resources have loaded before taking the screenshot, while maxRenderWait is the maximum total time allowed from the time the request starts before taking the screenshot. @maxwell can you confirm this is correct?

If so, then I believe I found the problem. The initialization of the maxRenderWait timer is occurring in the code being injected into the page, while the method being called is outside the page. That seems to be causing some sort of context / timing issue. I moved it out of the injected code, and it seems to have solved it. PR on its way.

christophersansone commented 9 years ago

@maxwell I have a related question about https://github.com/maxwell/screencap/blob/master/lib/screencap/raster.js#L161 . In my tests I have seen this line occur several times for the same request (e.g. http://www.ebay.com) ... the resource count will drop to zero and then become positive again if the resources have nested resources. If it would only occur once, it would make sense to clear maxRenderWait in favor of resourceWait, but that does not seem to be the case. As a result, I'd recommend removing this line and therefore force the screenshot after maxRenderWait milliseconds, no matter what.

Happy to submit a different issue / PR or just add it to this one... just let me know.

maxwell commented 9 years ago

@christophersansone not sure, @DEfusion added that line!