WIP: add support for orphan and widow page breaking #71
Conversation
|
Nice work! I especially liked the clone of the original object to check for overflow onto the next page. Adding appropriate unit tests will be the main way to ensure there is no regression. Adding tests for the new methods added should be relatively straightforward. But writing appropriate tests for |
…widows and orphans
|
So, what next? :-) |
|
There are a handful of issues remaining with multi-column processing that need to be addressed in this PR before it can be made ready for merging. Specifically with regards to processing of the test document, amnesty2014-report-english-litho-full.php. At least three specific problems have been identified and documented (via TODO comments only thus far). Unfortunately I will not have the time to dedicate my attention to these issues to make a speedy resolution, but I will focus on these as time permits. In the meantime, I'm available for questions / discussion if anyone else wants to have a go at addressing the remaining issues. |
|
What if for the initial patch you disabled orphan and widow support for multi-columns? From memory mPDF already disabled a couple of features when columns are enabled anyway. |
| @@ -120,12 +120,16 @@ | |||
|
|
|||
| // DEBUGGING & DEVELOPERS | |||
| $this->showStats = false; | |||
| $this->debug = false; | |||
| $this->debug = true; //false; | |||
There was a problem hiding this comment.
Good idea to debug by default in the development branch.
There was a problem hiding this comment.
Not so sure whether committing the changed value is a good idea at all. It may (and eventually will) lead to erroneous merging this to stable branches.
There was a problem hiding this comment.
That's a fair point. One way to handle this is to have a release script which sets reasonable defaults for production users.
There was a problem hiding this comment.
I'd rather see some sort of overridable local configs in 7.0+
|
Regarding the conflicts shown below, recent commits (since December 2015) to https://github.com/mpdf/mpdf/commits/development can be merged in, except for changing all "else if" to "elseif" but that is easily done. |
|
merged latest changes from development, and addressed issues with debug in default config. custom_config.php can contain any overrides desired, and has been added to .gitignore |
Create simpler examples for widow and orphan processing, use Composer
|
Thanks for all the work done on this! A first-sight issue I have with the code are spaces vs. tabs in the codebase, could you please change spaces to tabs? Thanks. Also, please, rebase your branch to current development. Custom config is a nice idea for the time being, though I would like to name the file I will test results of the patch and review the code more thoroughly then. |
…efault config.php
|
@finwe - thanks for your comments! your suggestions have been implemented. Please note, however, that the PR is still WIP and contains many unresolved issues and not at all in review state at the moment. Any help with the issues (see above comments, and TODO comments in mpdf.php) would be much appreciated as I will have very limited time to dedicate to this project. |
|
Ok, duly noted. I gained an impression from Daniel's comments that this could be somehow merged as a first phase. I won't, then. Anyway thanks a lot for the changes, although the rebase is still not entirely to my satisfaction, I would like to see a straight Git history line above current development. I'd rather have you make the rebase, because when I try, there are many conflicts that I am not entirely confident to solve properly. This is not a priority, though. |
|
actually last update was a rebase upstream/development, but usually I merge and squash commits at the end, but whatever you prefer is fine with me. btw, I'm not seeing any conflicts with latest upstream/development |
|
I personally think it would be good to merge this PR in the development branch, as the current lack of widows and orphans handling are a blocker for many potential users. We are already using Mark's version in production for single-column book layouts and it works well for those. |
|
closing this to satisfy finwe's rebase policy. will submit another PR with latest changes soon. |
|
It is entirely sufficient to rebase and/or squash and then force-push, the PR will update itself. Thanks, anyway! |
|
we still have some issues to resolve with this feature. new PR will be submitted hopefully this weekend. |
|
@thnkloud9 I am really looking forward to your PR. Thanks! |
|
I can't make head or tale out of this thread. I am not a programmer. So, is Orphan/Widow control added or not? And if yes, where can I download it? Sources of 6.1 do not include the keywords "minWidowLines" or "minOrphanLines" |
|
it has NOT been added and I have stopped work on my fork for this feature, sorry. |
|
Thanks for clarifying this. This is bad news for me as a working Widow/Orphan control is essential for professionell paper printing (I am a book/ebook publisher) |
|
If you are publishing in European languages try https://github.com/speedata/publisher instead, it has widow and orphan control and a very good layout engine. If you are publishing in Eastern languages such as Arabic or Chinese, mPDF has great support for these languages but Speedata Publisher does not. |
|
@danielhjames Sorry for reacting late. I wasn't (why ever) not informed by this thread. I will look this up, it sounds very promising, especially because I am German as the developer. |
|
Page linked to this work: https://dev.tiki.org/Widows-and-Orphans-in-mPDF |
To config min lines require before allowing a block orphan / widow update config.php:
$this->minWidowLines = 3;
$this->minOrphanLines = 3;
a value of 0 here should result in current behavior without auto breaks.
I have only so far tested with examples/example02_CSS_styles.php, and this is a bit of a hasty patch, so any help with testing, examples, and feedback (be critical, I can take it ;-) would be very much appreciated. I would be interested in the proper way to regression test and make sure I didn't break anything with these changes as well.