Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.3 linklist with label attribute prevents page loading #1890

Closed
phiw13 opened this issue Nov 25, 2023 · 24 comments
Closed

PHP 8.3 linklist with label attribute prevents page loading #1890

phiw13 opened this issue Nov 25, 2023 · 24 comments
Assignees
Milestone

Comments

@phiw13
Copy link

phiw13 commented Nov 25, 2023

With the default theme:

  • find the “body_aside” form (Misc. type),
  • add to the txp:linklist tag: wraptag="p" label="thisLABEL"
  • load or reload the front page. Watch the browser progress bar (or spinning icon) stop and you wait and you wait. after a long wait, a browser message: server too busy, connection reset (see this in the forum: https://forum.textpattern.com/viewtopic.php?pid=335896#p335896) but no page…

The httpd error log has this (localhost + apache):

Fatal error: Maximum execution time of 30+2 seconds exceeded (terminated) in /Users/username/Sites/testbed/textpattern/lib/txplib_misc.php on line 1833

This worked fine with the exact same configuration and PHP 8.2.

Note:
It is possible that other TXP tags combined with this label attribute exhibit the same problem, but so far I haven’t seen it.

@jools-r
Copy link
Sponsor Member

jools-r commented Nov 25, 2023

Some functions use the doLabel function and others leave it to be handled by the tag-wide txp_wraptag function. linklist does the latter.

Presumably, one could fix this by modifying this line to read:

return $out ? ($label ? doLabel($label, $labeltag) : '') . doWrap($out, $wraptag, $break, $class) : '';

and adding "labeltag" and "label" as attributes in the linklist attributes.

But this didn't exist beforehand and works in php 8.1 / txp 4.8.8, so maybe the problem lies with txp_wraptag or the registerAttr attribute handling functionality?

@bloatware
Copy link
Member

Looks like php 8.3 bug for me. As soon as I comment //unset($txp_atts[$name]); in lAtts() function (l. 1833) and empty global txp_atts array instead, things go better. But what is wrong about unset($txp_atts[$name]);???

bloatware added a commit that referenced this issue Nov 25, 2023
@jools-r
Copy link
Sponsor Member

jools-r commented Nov 26, 2023

Works for me with PHP 8.3 on local homebrew setup. Thank you! 👍

@Bloke
Copy link
Member

Bloke commented Nov 26, 2023

Can't see any reason for the change in behaviour in the PHP 8.3 release notes or migration guide. Weird.

@phiw13
Copy link
Author

phiw13 commented Nov 26, 2023

This seems to work now, local Homebrew setup with both a default install and the old-old-old testbed&playground. So far I haven’t seen any unexpected side-effects.

@bloatware
Copy link
Member

This looks like a weird internal pointer bug. This worked:

<txp:section_list label="sections" labeltag="h2" wraptag="ul" break="li" />

but not this:

<txp:section_list wraptag="ul" break="li" label="sections" labeltag="h2" />

In both cases we are unsetting $txp_atts array elements (some of which are referenced), but in a different order. I can not reproduce the issue outside of txp, though.

Pity, now we have to duplicate the array on loops, which is waist of cycles. But there is a chance php 8.3.1 will be released before txp 4.9 ;-)

@bloatware
Copy link
Member

I have narrowed it to a bare minimum. This (ok, weird) code works in php 8.2 but halts in 8.3:

<?php
$test = array(
  'wraptag' => 'ul',
  'break' => 'li',
  'label' => 'sections',
  'labeltag' => 'h2',
);

unset($test['wraptag']);
unset($test['break']);
//unset($test['label']);
//unset($test['labeltag']);

foreach($test as $k => &$v) {// mind the reference
    foreach($test as $kk => $vv) unset($test[$kk]);
}

unset($v);
var_dump($test); //empty array

Amazingly, if we unset only, say, $test['break'] before entering the loop, everything is fine. It's fine also if we loop by value $k => $v, whatever keys pre-unset. My php-fu is ko.

Don't try on a live php 8.3 server, it could bite.

@bloatware
Copy link
Member

PHP 8.3.0 bug confirmed and fixed. That means that current txp versions are not usable on 8.3.0, but will be again on 8.3.1. Should we adapt (degrade) txp 4.9 core code just for 8.3.0 compatibility? Would be pity...

@Bloke
Copy link
Member

Bloke commented Dec 2, 2023

Nice work flagging it to PHP devs. And thank you, phiw13 for spotting this.

Anybody have any intel on when 8.3.1 is expected?

@Bloke
Copy link
Member

Bloke commented Dec 2, 2023

P.S. We should note this in our HISTORY or README.md that Txp will not run on 8.3.0 under some circumstances.

@phiw13
Copy link
Author

phiw13 commented Dec 2, 2023

Nice work @bloatware !

Of course you should not hurt Textpattern for this.

Anybody have any intel on when 8.3.1 is expected?

I guess in 3 weeks or so (based on their usual pattern) @petecooper would know for sure.

(don’t worry about my testbed site, this issue is actually easy to work around)

@petecooper
Copy link
Member

Anybody have any intel on when 8.3.1 is expected?

RC is tagged Dec 5th, GA is tagged Dec 19th.

Assuming no deviation from regular release schedule.

@petecooper
Copy link
Member

Is this our first official PHP bug report that's been triaged and fixed?

@bloatware
Copy link
Member

Is this our first official PHP bug report that's been triaged and fixed?

Nice to see they have fixed it so quickly.

@petecooper
Copy link
Member

Anybody have any intel on when 8.3.1 is expected?

8.3.1 RC1 compiled and ready.

@bloatware
Copy link
Member

8.3.1 RC1 compiled and ready.

Thanks @petecooper! As soon as it is released, we'll skip 8.3.0 compatibility.

@petecooper
Copy link
Member

ETA: tagged Dec 19 2023, released Dec 21 2023.

@petecooper
Copy link
Member

petecooper commented Dec 6, 2023

PHP 8.3.1RC1-clean (¯\_(ツ)_/¯) compiling now.

https://github.com/php/php-src/releases/tag/php-8.3.1RC1-clean

@petecooper
Copy link
Member

petecooper commented Dec 6, 2023

No sooner had I compiled PHP 8.3.1RC2 (supersedes PHP 8.3.1RC1-clean), I got a note about PHP 8.3.1RC3 being released…so, now I'm compiling PHP 8.3.1RC3. Maybe we'll have RC4 before bedtime…

I can't recall any patch release getting to RC3 before…something's going on at PHP Towers, clearly

@petecooper
Copy link
Member

PHP 8.3.1 compiling now.

@petecooper
Copy link
Member

Paging @bloatware for completeness – is this resolved from your perspective, please?

@bloatware
Copy link
Member

@petecooper not exactly, I haven't yet installed php 8.3.1 :-) Will do in a due course, thanks for the reminder.

@petecooper
Copy link
Member

@bloatware PHP 8.3.1 is installed to dev-demo if that saves you some housekeeping.

@bloatware
Copy link
Member

@petecooper yep, and that's great. But I prefer local testing first, before pushing txp changes to the repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants