Skip to content
This repository has been archived by the owner on May 30, 2023. It is now read-only.

Improve printing #211

Closed
wants to merge 3 commits into from
Closed

Improve printing #211

wants to merge 3 commits into from

Conversation

milianw
Copy link
Contributor

@milianw milianw commented Feb 28, 2012

here are three patches, I'd like to upstream.

the first one fixes a build-issue when using build-linux.sh with --headless - it is not really related to the other two, but I was to lazy to create a separate pull request just for that... hope you don't mind

then there is one commit that simplifies the handling of qt-patches in the build-helper.sh, namely the patch files are now read from folders. that way, a user can just dump custom patches there and run the deploy script without changes.

finally, there are three new qt 4.8 patches, that leverage this new patch-folder structure. the patches are improve PhantomJS printing capabilities substantially. Most notably, tables are now printed much nicer (thead, tfoot are repeated and there are no more page-breaks inside table rows). Furthermore users can add a custom header/footer to pages by adding some javascript to their page(s):

    <script type="text/javascript">
    var PhantomJSPrinting = {
      // supported units: mm, cm, in, px
      headerHeight : function() { return '1cm'; },
      footerHeight : function() { return '1cm'; },
      // just return any kind of html that make up the header/footer
      // it should not be taller than what is returned in header/footerHeight()
      header: function(currentPage, totalPages) { return currentPage + " / " + totalPages; },
      footer: function(currentPage, totalPages) { return currentPage + " / " + totalPages; }
    };
    </script>

in such cases, DISABLE_HEADLESS was unset and hence
the expansion -a -eq 0 resulted in a bash error:

bash: [: too many arguments

this is fixed now by using string-based comparison
generic patches now reside in deploy/qt-patches/all
while 4.8 patches are now in deploy/qt-patches/4.8

all *.patch files in these folders are applied during deployment
the webpage needs to define the following java script functions:

<script type="text/javascript">
var PhantomJSPrinting = {
  // supported units: mm, cm, in, px
  headerHeight : function() { return '1cm'; },
  footerHeight : function() { return '1cm'; },
  // just return any kind of html that make up the header/footer
  // it should not be taller than what is returned in header/footerHeight()
  header: function(currentPage, totalPages) { return currentPage + " / " + totalPages; },
  footer: function(currentPage, totalPages) { return currentPage + " / " + totalPages; }
};
</script>

The page-counter can be reset by adding the class "phantomjs_reset_pagination"
attribute to html block-elements that should reset the counter.

page-break-inside:avoid is now properly honored by block elements

thead and tfoot of tables are now repeated on every page the table spans

rows are layouted such that a page-break is avoided inside them
@milianw
Copy link
Contributor Author

milianw commented Feb 28, 2012

you can see an example .html & .pdf file here:

https://userpage.physik.fu-berlin.de/~milianw/phantomjs/printing/

@ariya
Copy link
Owner

ariya commented Feb 28, 2012

For reasons described elsewhere, I rather have the issue linked from the commit log. But if you don't have time to add them, I'll try to amend the commit message as I cherry-pick it.

@milianw
Copy link
Contributor Author

milianw commented Feb 28, 2012

is there a issue for this? I haven't found one.

I'd welcome it if you could just add the "issue: ..." line yourself - sorry for the inconvenience!

@ariya
Copy link
Owner

ariya commented Mar 1, 2012

Well yeah, technically a feature should be filed in the issue tracker as well (per our Contribution Guide).

@milianw
Copy link
Contributor Author

milianw commented Mar 2, 2012

so, should I create a new issue? I found http://code.google.com/p/phantomjs/issues/detail?id=63 which is I think at least partially fixed by this - I'd of course also be ok with you just merging these patches with the "issue: ..." line added.

many thanks

@ariya
Copy link
Owner

ariya commented Mar 2, 2012

Sure, 63 seems to be a representative issue which I can use. Thanks!

@ariya
Copy link
Owner

ariya commented Mar 5, 2012

I'm going to hold off merging the printing patch that requires global object (that's not a good practice). Once we have a better JS binding machinery, this will be better handled.

This is also a good reason to prepare an issue in the tracker which discuss the problem/solution.

@milianw
Copy link
Contributor Author

milianw commented Mar 8, 2012

see http://code.google.com/p/phantomjs/issues/detail?id=410&start=100 - could you please elaborate on which "global object" you mean?

@milianw
Copy link
Contributor Author

milianw commented Apr 11, 2012

partially merged, rest is going to appear in a new mr soon

@milianw milianw closed this Apr 11, 2012
@milianw milianw mentioned this pull request Nov 22, 2012
ariya pushed a commit that referenced this pull request Jan 7, 2014
This patch is taken from https://bugs.webkit.org/show_bug.cgi?id=5097#c17

It was originally part of PR #211 but was possibly overlooked in PR #344
(when the other two patches got reapplied after the QT source import).
trygveaa added a commit to trygveaa/phantomjs that referenced this pull request Jun 16, 2015
This has been done a few times before[0][1], but seems to be broken
again as reported in [2].

This commit is only reinstating what's missing from [1]. This was done
by running:

    git log -p -1 2d778f6 | git apply -3 -p6 --directory=src/qt/qtwebkit

And resolving the conflicts. There are still some compile errors. These
will be fixed in the upcoming commits instead of in this one to clarify
the changes.

This change doesn't completely fix the issue. The thead/tfoot block is
drawn again on the next page, however the other content is not shifted
down accordingly. This means that the thead/tfoot block on the next page
will be placed on top of the continued table instead of above it.

[0]: ariya#211
[1]: ariya#344
[2]: ariya#13324
trygveaa added a commit to trygveaa/phantomjs that referenced this pull request Jun 16, 2015
This has been done a few times before[0][1], but seems to be broken
again as reported in [2].

This commit is only reinstating what's missing from [1]. This was done
by running:

    git log -p -1 2d778f6 | git apply -3 -p6 --directory=src/qt/qtwebkit

And resolving the conflicts. There are still some compile errors. These
will be fixed in the upcoming commits instead of in this one to clarify
the changes.

This change doesn't completely fix the issue. The thead block is drawn
again on the next page, however the other content is not shifted down
accordingly. This means that the thead block on the next page will be
placed on top of the continued table instead of above it. The tfoot
block is also repeated and seems to work as expected.

[0]: ariya#211
[1]: ariya#344
[2]: ariya#13324
trygveaa added a commit to trygveaa/phantomjs that referenced this pull request Jun 16, 2015
This has been done a few times before[0][1], but seems to be broken
again as reported in [2].

This commit is only reinstating what's missing from [1]. This was done
by running:

    git log -p -1 2d778f6 | git apply -3 -p6 --directory=src/qt/qtwebkit

And resolving the conflicts. There are still some compile errors. These
will be fixed in the upcoming commits instead of in this one to clarify
the changes.

[0]: ariya#211
[1]: ariya#344
[2]: ariya#13324
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants