bithost-gmbh / pdfviewhelpers

TYPO3 CMS extension that provides various Fluid ViewHelpers to generate PDF documents.
GNU General Public License v3.0
44 stars 21 forks source link

HTML viewhelper should allow CSS files from other extension's Private folder #247

Open liayn opened 1 month ago

liayn commented 1 month ago

Describe the bug

It is not possible to use CSS files for HTMLViewHelper, which are located in a Resources/Private folder of an extension.

Environment TYPO3 version(s): v12+ pdfviewhelpers version: v3.0.0

Steps to reproduce

Configure in TypoScript:

styleSheet = EXT:myext/Resources/Private/some/path/styles.css

Expected behavior

The stylesheet is loaded.

Possible solution

Adjust the code part in https://github.com/bithost-gmbh/pdfviewhelpers/blob/main/Classes/ViewHelpers/HtmlViewHelper.php#L97 to:

if (!empty($this->arguments['styleSheet'])) {
            try {
                $styleSheetFile = $this->conversionService->convertFileSrcToFileObject($this->arguments['styleSheet']);
                $styleSheetContent = $styleSheetFile->getContents();
            } catch (ValidationException $e) {
                $styleSheetFile = GeneralUtility::getFileAbsFileName($this->arguments['styleSheet']);
                if (!$styleSheetFile) {
                    throw $e;
                }
                $styleSheetContent = file_get_contents($styleSheetFile);
            }

            $htmlStyle = '<style>' . $styleSheetContent . '</style>';
        }

If the CSS file can't be found via FAL, try directly loading it.

maechler commented 1 month ago

@liayn Thank you very much for that detailed bug report and providing a fix! I'll need to have a closer look at this use case and the failing test, but probably the solution will be to adjust the test logic. I can take care of this, let me know in case you'd need a quick fix.

liayn commented 1 month ago

The use case is rather simple: Nothing should be public that does not need to be public. In case of CSS for a PDF: No need to go public.

(Actually this is basically the same for every local resource used when rendering PDF.)

maechler commented 1 month ago

Yes, I totally agree with the benefits of this! I was more concerned with the failing test, because with this change we do not throw a ValidationException anymore when the file does not exist. Thinking more about it, I would replace the exception rethrow with the following to fix the test and stay consistent with the current error handling:

if (!is_file($styleSheetFile)) {
    throw new ValidationException('Provided styleSheet file path "' . $styleSheetFile . '" does not exist. ERROR: 1729546062', 1729546062, $e);
}

What do you think?

liayn commented 1 month ago

We should probably use PathUtility::isExtensionPath($persistenceIdentifier) first. There is no need to go through FAL at all, if a resource stems from an extension.