Conversation
We now have a few classes that separate the concerns
DokuPDF - inherits from mPDF and doesn't do much anymore except for a
few internal defaults and overwrites
Config - encapsulate the configuration (file and input vars) and created
an mpdf config array
Template - handles loading the template files and applies placeholders
Writer - composites the above classed and gives access to write
operations
We now have a Styles class that takes care of loading the styles from plugins etc.
The basic handling of page collecting, caching and rendering is now defined. There is still some ugly dependency in the renderer and the ImageProcessor loading is not reimplemented yet. After the above a first attempt to actually run the code can be made.
This should make tests and a future command line interface easier to implement.
As suggested in mpdf/mpdf#2145 the previous decorator functionality was implemented via custom HTTPClient and LocalContent loaders.
We want the ID to be passed via Config as well.
These tests have been initially LLM written, I went through each of them to make sure they make sense and fixed what I thought needed fixing. However coverage is still not great and should be improved in the future.
This is now handled within the renderer itself
Now only event handling happens in the action handler
|
Impressive refactoring! Thanks for the work! Just a first small observation. The tpl url-parameter seems now not supported anymore? Is that intended? I have reports that people seem to use it. I was still considering to add a dropdown or something like that for it in the Bookcreator. |
Hmm you're right. Do you think this is a security issue? Might be a SEO issue at least if debug links end up on the web (duplicate content). BTW. I am not 100% happy with how debugging currently works anyway (just suddenly dumping HTML and exiting). I might work on it a bit more. |
I do not expect it will expose css or html people could otherwise not find
Its purpose is to have a way to grab the entire input that is given to mPDF, to enable debugging pdf creation itself. Since we add page for page to mPDF this workaround with buffering html is added. Of course, it is not the literal input to mPDF. |
This should make it easier to add new configuration and see with a glance how it is initialized.
This refactors the service so that tests can easily access the produced debug HTML
The test passes, but I am not 100% sure the behaviour is correct yet. Static variables in the renderer might be a problem. Not tested with multiple documents yet. Currently render options need to be set via global conf and are not passed from the Config object.
Using static variables makes it impossible to automatically test this correctly. The PDF renderer is now a singleton. The Writer class initializes it anew for each export, but during the export, the same renderer is reused.
This streamlines the configuration to a single point of entry. The renderer was the last bit not adhering to that.
Again mostly LLM generated but manually checked and corrected.
|
Merged the attributes PR, added a whole bunch of tests, fixed issues with numbered headers in the renderer. |
| $this->debug = $config->isDebugEnabled(); | ||
|
|
||
| // initialize a new renderer instance (singleton instance will be reused in later p_* calls) | ||
| $renderer = plugin_load('renderer', 'dw2pdf', true); |
There was a problem hiding this comment.
Could this have influence on rendering included page by Include plugin? Or other render specialities?
There was a problem hiding this comment.
I don't think so. This should not be too different from the combination of a reference to the (singleton) action plugin and class static variables that were used before.
mpdf v8.2.6 => v8.2.7 log 3.0.2 => 1.1.4 Using the old log version avoids conflicts with the smtp plugin for now. However we need a proper solution to this problem in the future (probably in core). A PR to be able to use any log version with the smtp plugin has been submitted: txthinking/Mailer#36 We probably should introduce a log/psr:3 dependency in core when upgrading to PHP8 (and provide a PSR compatible wrapper around our logger)
Note that i accidentally ran and pushed this on master as well. it's interesting that the issue seemingly did exist on master.
This should have been added in the last commit 837fcad but was forgotten.
deleted.files includes DokuPDF already, and on some systems the extension manager might delete DokuPdf as well
Use latest mpdf and custom AssetFetcher to fix media paths
Fix revision handling
Set up book title to be available for placeholder replacement
3 participants
This is a major refactoring of the plugin's code base with the goal to make it easier to maintain and extend. The monolithic architecture has been broken into several classes with clearly separated concerns.
All PDF generation is now decoupled from the environment. This is in preparation to add a a command line or remote API component in the future.
MPDF has been updated to version 8.2.
Service classes:
New collector architecture is responsible to collect the pages to be added to the PDF.
DW2PDF_NAMESPACEEXPORT_SORTwhere plugins could influence the sorting (or even filter out pages)Media Handling #527
Link rewriting is now done outside the (cached) rendering. The plugin requires the DOM and XML extension for this.
A bunch of tests have been added but they are far from comprehensive. Help in manually testing this branch would be very welcome
In theory, bookcreator integration should remain unchanged, but has not been tested. I did however change a lot of the Exception handling regarding the bookcreator integration so this part might behave different.
Issues addressed in this PR: #528 #527 #526 #508 #455 #140 (could be implemented via the new event now)