Since PHP 8.1, passing a null string to ltrim(string $string) is deprecated.
Thus first check if After and Below text are not null before trimming.
Closes T15379
Details
Details
- Reviewers
valerio.bozzolan - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15379: PHP 8.1 "ltrim(null)" exception blocks rendering first Meme comment
- Commits
- rPb4cfe56f03b4: Fix PHP 8.1 "ltrim(null)" exception which blocks rendering first Meme comment
Applied this change; afterwards several times created a new Pholio mock and added a Meme comment. I could not reproduce the problem anymore (first meme comment in a mock always rendered correctly instead of showing an exception as comment). However, I could not reliably reproduce the problem anyway.
- Create a meme without any above/below text
- Create a meme with just spaces as above/below text
- Create a meme with just above text as "asd", or "0" or a lizard
- Create a meme with just below text as "asd", or "0" or a lizard
- Create a meme with both above and below texts with "asd", "0" and a lizard and more stuff (doing all 64 combinations)
Diff Detail
Diff Detail
- Repository
- rP Phorge
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 389 Build 389: arc lint + arc unit
Event Timeline
Comment Actions
Thanks for addressing this issue
We still need to work on this since, before this change this was allowed:
{meme, src=somememe, above=0, below=0}
But after this change, the resulting meme has no above/below texts.
(Note: it's not so easy to test what I'm saying, since memes are cached from the above/below fingerprint, so, to test this locally, we have to upload another meme for each test to be conducted)
| src/applications/macro/engine/PhabricatorMemeEngine.php | ||
|---|---|---|
| 185–186 | Probably this would allow more memes | |
