voku / HtmlMin

:clamp: HtmlMin: HTML Compressor and Minifier via PHP
MIT License
160 stars 21 forks source link

Uncaught TypeError with PHP 8.3 and `id` attribute #93

Open lee-peuker opened 6 months ago

lee-peuker commented 6 months ago

What is this feature about (expected vs actual behaviour)?

Running into an error after when minifying <div id="test"></div> after updating to PHP 8.3. I expected to get the minified html.

How can I reproduce it?

Script to reproduce issue (using PHP 8.3.0):

<?php declare(strict_types=1);

use voku\helper\HtmlMin;

include_once(__DIR__ . '/../vendor/autoload.php');

$vokuMinifier = new HtmlMin;

$vokuMinifier->minify('<div id="test"></div>');

Error:

Fatal error: Uncaught TypeError: Cannot assign null to property DOMElement::$id of type string in /vendor/voku/simple_html_dom/src/voku/helper/AbstractSimpleHtmlDom.php:174
Stack trace:
#0 /vendor/voku/html-min/src/voku/helper/HtmlMinDomObserverOptimizeAttributes.php(146): voku\helper\AbstractSimpleHtmlDom->__set('id', NULL)
#1 /vendor/voku/html-min/src/voku/helper/HtmlMin.php(1688): voku\helper\HtmlMinDomObserverOptimizeAttributes->domElementAfterMinification(Object(voku\helper\SimpleHtmlDom), Object(voku\helper\HtmlMin))
#2 /vendor/voku/html-min/src/voku/helper/HtmlMin.php(1667): voku\helper\HtmlMin->notifyObserversAboutDomElementAfterMinification(Object(voku\helper\SimpleHtmlDom))
#3 /vendor/voku/html-min/src/voku/helper/HtmlMin.php(1370): voku\helper\HtmlMin->minifyHtmlDom('<div id="test">...', false)
#4 /scripts/htmlMin.php(9): voku\helper\HtmlMin->minify('<div id="test">...')
#5 {main}
  thrown in /vendor/voku/simple_html_dom/src/voku/helper/AbstractSimpleHtmlDom.php on line 174

Does it take minutes, hours or days to fix?

No idea

Any additional information?

in AbstractSimpleHtmlDom.php:174 there already seems to be some catch in place, if this is extended with $nameOrig === 'id' the error is fixed, but not sure if this is wanted.

                    // INFO: Cannot assign null to property DOMNode::* of type string
                    if ($nameOrig === 'prefix' || $nameOrig === 'textContent') {
                        $value = (string)$value;
                    }
ArnaudLigny commented 6 months ago

Hello, I'm also trying to update my app to PHP 8.3 and got the same problem:

TypeError: Cannot assign null to property DOMElement::$id of type string

/home/runner/work/Cecil/Cecil/vendor/voku/simple_html_dom/src/voku/helper/AbstractSimpleHtmlDom.php:174
/home/runner/work/Cecil/Cecil/vendor/voku/html-min/src/voku/helper/HtmlMinDomObserverOptimizeAttributes.php:146
/home/runner/work/Cecil/Cecil/vendor/voku/html-min/src/voku/helper/HtmlMin.php:1688
/home/runner/work/Cecil/Cecil/vendor/voku/html-min/src/voku/helper/HtmlMin.php:1667
/home/runner/work/Cecil/Cecil/vendor/voku/html-min/src/voku/helper/HtmlMin.php:1370

https://github.com/Cecilapp/Cecil/actions/runs/7120600738/job/19388168813?pr=1676#step:14:659

repat commented 6 months ago

@lee-peuker @ArnaudLigny The error is in the underlying package voku/simple_html_dom, not in this package.

@voku There is already a fix PR for it: https://github.com/voku/simple_html_dom/pull/106 Could you please merge it so we can use your package in PHP 8.3? :) Thanks!

ConnectGrid commented 5 months ago

While I agree that the SimpleHtmlDom needs the fix, shouldn't this one "watch" for such violations, too? Should it not detect if id was trying to be assigned a value of null, rather than just blindly trying to set every attribute to null?

The null assignment is happening on Line 146 in the voku\helper\HtmlMinDomObserverOptimizeAttributes\domElementAfterMinification() method of this library, which is why it's easy to think that this library needs the fix.

thierrydrc commented 3 months ago

Hope that the merge will be done soon 🙏