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

wkhtmltoimage ignores --javascript-delay and --window-status #2142

Closed
mo-gr opened this Issue Jan 12, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@mo-gr

mo-gr commented Jan 12, 2015

Setting either --javascript-delay or --window-status has no effect in wkhtmltoimage. Regardless of the parameter value, the image seems to be always generated immediately. (wkhtmltopdf works as expected).

This is a problem for us because javascript based page setup is not complete when wkhtmltoimage generates the image.

@ashkulz

This comment has been minimized.

Show comment
Hide comment
@ashkulz

ashkulz Jan 12, 2015

Member

Version? Platform? Example HTML/CSS which reproduces this issue?

Member

ashkulz commented Jan 12, 2015

Version? Platform? Example HTML/CSS which reproduces this issue?

@mo-gr

This comment has been minimized.

Show comment
Hide comment
@mo-gr

mo-gr Jan 12, 2015

Sorry for not giving you this info right away.

I can reproduce this with the 12.2 binaries for OSX (64 bit) and the official linux rpm builds.

Reproducible with the following html

<body>
  <div id="main">
    foo
  </div>
  <script type="text/javascript" charset="utf-8">
    setTimeout(function(){
      document.getElementById('main').innerHTML = "bar";
    }, 1000);
  </script>
</body>
</html>

and the following command line

wkhtmltoimage --javascript-delay 2000 index.html bar.png

I would expect the png to contain "bar" but it contains "foo"

mo-gr commented Jan 12, 2015

Sorry for not giving you this info right away.

I can reproduce this with the 12.2 binaries for OSX (64 bit) and the official linux rpm builds.

Reproducible with the following html

<body>
  <div id="main">
    foo
  </div>
  <script type="text/javascript" charset="utf-8">
    setTimeout(function(){
      document.getElementById('main').innerHTML = "bar";
    }, 1000);
  </script>
</body>
</html>

and the following command line

wkhtmltoimage --javascript-delay 2000 index.html bar.png

I would expect the png to contain "bar" but it contains "foo"

mo-gr pushed a commit to mo-gr/wkhtmltopdf that referenced this issue Jan 12, 2015

Moritz Grauel
fixes #2142
To be quite honest, I'm really not aware of any possible side-effects of
turning the loader into a mainLoader.

But it does fix #2142. wkhtmltoimage now respects `--javascript-delay`
and `--window-status` commandline flags.

So far, in all our tests on debian-linux, wkhtmltoimage works and
behaves as expected when the MultiPageLoader is configured as a
mainLoader as by this patch.

@mo-gr mo-gr referenced this issue Jan 12, 2015

Closed

fixes #2142 #2144

@mo-gr

This comment has been minimized.

Show comment
Hide comment
@mo-gr

mo-gr Jan 12, 2015

Apparently the loader is not configured as a mainLoader in wkhtmltoimage. Therefore --javascript-delay and --window-status are not evaluated in ResourceObject::loadFinished(). I've patched the image converter to be a mainLoader and wkhtmltoimage works as expected. But the project is too complex for me to be sure, that this doesn't have any unwanted side effects.

So be a bit careful with the pull request ;-)

mo-gr commented Jan 12, 2015

Apparently the loader is not configured as a mainLoader in wkhtmltoimage. Therefore --javascript-delay and --window-status are not evaluated in ResourceObject::loadFinished(). I've patched the image converter to be a mainLoader and wkhtmltoimage works as expected. But the project is too complex for me to be sure, that this doesn't have any unwanted side effects.

So be a bit careful with the pull request ;-)

@ashkulz

This comment has been minimized.

Show comment
Hide comment
@ashkulz

ashkulz Jan 12, 2015

Member

PR looks OK, this is a regression because of #1892. Good catch!

Member

ashkulz commented Jan 12, 2015

PR looks OK, this is a regression because of #1892. Good catch!

@ashkulz ashkulz added the Verified label Jan 12, 2015

@stratosilva

This comment has been minimized.

Show comment
Hide comment
@stratosilva

stratosilva Jan 13, 2015

I have exactly the same problem described and I really need the fix urgently. I currently use the Windows x64 version and I have no way to build it myself, so I would appreciate if you could share with me somehow the files I need to patch to make --javascript-delay work for wkhtmltoimage. Thanks

stratosilva commented Jan 13, 2015

I have exactly the same problem described and I really need the fix urgently. I currently use the Windows x64 version and I have no way to build it myself, so I would appreciate if you could share with me somehow the files I need to patch to make --javascript-delay work for wkhtmltoimage. Thanks

@ashkulz

This comment has been minimized.

Show comment
Hide comment
@ashkulz

ashkulz Jan 14, 2015

Member

The instructions on how to build your version are crystal clear and require only freely available tools. If it is that urgent, you should try to do it yourself. After all, this is an open source project 😄

Member

ashkulz commented Jan 14, 2015

The instructions on how to build your version are crystal clear and require only freely available tools. If it is that urgent, you should try to do it yourself. After all, this is an open source project 😄

@stratosilva

This comment has been minimized.

Show comment
Hide comment
@stratosilva

stratosilva Jan 14, 2015

Indeed I could, but that would take time... Something scarce at present (I am involved in an Ebola Emergency project within my organization). I thought that it would be just one click to build it for someone who has all development environment ready, that's the reason I asked. In any case, I downloaded version 0.11 and it seems to work now, so it is not that urgent anymore. Thanks.

stratosilva commented Jan 14, 2015

Indeed I could, but that would take time... Something scarce at present (I am involved in an Ebola Emergency project within my organization). I thought that it would be just one click to build it for someone who has all development environment ready, that's the reason I asked. In any case, I downloaded version 0.11 and it seems to work now, so it is not that urgent anymore. Thanks.

@ashkulz

This comment has been minimized.

Show comment
Hide comment
@ashkulz

ashkulz Jan 14, 2015

Member

@stratosilva: use 0.12.1.2 instead, but it won't have the latest OpenSSL.

Member

ashkulz commented Jan 14, 2015

@stratosilva: use 0.12.1.2 instead, but it won't have the latest OpenSSL.

@ashkulz ashkulz closed this in f77d32f Jan 14, 2015

@ashkulz ashkulz added Fixed and removed Verified labels Jan 14, 2015

@ashkulz ashkulz added this to the 0.12.2.1 milestone Jan 14, 2015

@ashkulz

This comment has been minimized.

Show comment
Hide comment
@ashkulz

ashkulz Jan 20, 2015

Member

0.12.2.1 has been released, which includes changes related to this issue.

Member

ashkulz commented Jan 20, 2015

0.12.2.1 has been released, which includes changes related to this issue.

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