User tests: Successful: Unsuccessful:
Pull Request for Issue #41183 .
The banners module supports using banners by tag search or keywords. When the banners model is building the query for it, there is a reg-expression to search for words. This regex seems not be compatible with MySql version 8.0.4 and newer.
The SQL query, which is created, is something like: Where metakey regexp('[[:<:]]Mo[[:>:]]')
"Mo" is the keyword to search for.
Executing it on MySql 8.0.4 or any newer version, it throws the error "Illegal argument to a regular expression.".
I think the correct reg-expression must be: Where metakey regexp('\\bMo\\b')
In the MySql manual https://dev.mysql.com/doc/refman/8.0/en/regexp.html there is the explanation about the switch from the Henry Spencer's implementation to the ICU compatible implementation:
MySQL implements regular expression support using International Components for Unicode (ICU), which provides full Unicode support and is multibyte safe. (Prior to MySQL 8.0.4, MySQL used Henry Spencer's implementation of regular expressions, which operates in byte-wise fashion and is not multibyte safe.
Further information in the web can for example be found here:
https://bugs.mysql.com/bug.php?id=106504
https://stackoverflow.com/questions/59998409/error-code-3685-illegal-argument-to-a-regular-expression
System information:
Joomla: The site which had/has those issues had version 5 and is in the meantime migrated to 6 (where the errors still appear).
DB type: mysql
DB version: 8.0.42
PHP-Version: 8.4.7
Background information:
I use a custom component which calls in a view $this->getDocument()->setMetaData('keywords', $currentKeywords);
With this approach, the banners module gets those keywords in its BannersHelper method getBanners() line 45 with:
$keywords = explode(',', $app->getDocument()->getMetaData('keywords'));
With this Pull Request the reg-expression will be surrounded with a db type check, which makes sure the correct expression is used.
In the same part of the banners model there is
Deprecated
: str_starts_with(): Passing null to parameter #2 ($needle) of type string is deprecated in
E:\joomla\github\joomla-cms_5.4\components\com_banners\src\Model\BannersModel.php
on line
219
Warning
: foreach() argument must be of type array|object, false given in
E:\joomla\github\joomla-cms_5.4\components\com_banners\src\Model\BannersModel.php
on line
275
Honestly, I don't know how I can provide the keywords with the Joomla standard components. So the easiest way to reproduce is:
$keywords = explode(',', $app->getDocument()->getMetaData('keywords'));$keywords = ['Mo'];Deprecated
: str_starts_with(): Passing null to parameter #2 ($needle) of type string is deprecated in
E:\joomla\github\joomla-cms_5.4\components\com_banners\src\Model\BannersModel.php
on line
219
Warning
: foreach() argument must be of type array|object, false given in
E:\joomla\github\joomla-cms_5.4\components\com_banners\src\Model\BannersModel.php
on line
275
Warning: foreach() argument must be of type array|object, false given in E:\joomla\github\joomla-cms_5.4\modules\mod_banners\tmpl\default.php on line 27
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
| Category | ⇒ | Front End com_banners |
| Labels |
Added:
PR-5.4-dev
|
||
Good point, I didn't saw it. Yes, indeed, the word boundary part is the same. Although the related issue 41183 mentioned MariaDb as well and there is a discussion about a more generic/clean implementation. It seems that my PR is more like a quick fix.
@richard67 Since you were involved in the discussion of issue 41183, what do you think: Is my quick fix acceptable? I don't believe I'm capable of implementing the clean approach of extending the database providers.
| Title |
|
||||||
@richard67 Since you were involved in the discussion of issue 41183, what do you think: Is my quick fix acceptable? I don't believe I'm capable of implementing the clean approach of extending the database providers.
@beni71 If you accept my code change suggestions (can be done with buttons on GitHub), the PR is ok and can be changed from draft to ready for review.
I have now a general question:
It seems that this PR is based on an old way to pass keywords to the banners module, throught the document, which was removed from the Joomla components with #25258. Joomla components no longer support it. Does it mean that it is deprecated anyway? If so, I will close the PR.
To be honest: I have no idea if it is deprecated. Let's see if other comments come it.
The reason why I haven't made a PR yet for fixing the issue was because I was not able to find out how and when keywords were used in the banner module.
As far as I know keywords of articles are used for the related articles module.
Thanks, I will give it a try.
@beni71 - first off, thank you for your contribution, unfortunately, I am not getting exactly what you are intending:
(just a small, perhaps obvious instruction update , you need to set a position for the module ;))
I set mine to Below_Top (and then Main_Bottom to see if that would change things... only difference is that with Main_Bottom I saw the Chrome with the title: Site Module Banner which isn't present in Below_top).
BEFORE CONDITION, I only match 2 out of 4 of your conditions,
I get "No banners are visualized." - I do NOT have the error message: The getItems method has catched the error "Illegal argument to a regular expression." but returned
The PHP 8 deprecation warning "...Passing null to parameter #2 ($needle) of type string is deprecated..." is displayed.
I get:
Deprecated
: str_starts_with(): Passing null to parameter #2 ($needle) of type string is deprecated in
/home/lights/public_html/_j540/components/com_banners/src/Model/BannersModel.php
on line
219
which seems to be a match!
AFTER CONDITION: no more error message BUT no banner displayed. NO errors in Console in either case ;)
Hope that helps you!
| Labels |
Added:
bug
|
||
@beni71 There is no need to update your branch when it is shown as not up to date. It only needs to be done when there are conflicts shown. No problem now, but if you PR would already have successful human tests, the test counter would be reset by the branch update, and that could cause us missing it when searching for PRs which have 2 successful tests for setting them RTC (ready to commit).
As said, not a problem right now, I just wanted to explain for future use.
@exlemor Thank you for your test and feedback. You are right, the reproduction steps were not clear enough and some steps were missing. I have now started from a clean Joomla 5.4 (from the branch) and have revised the instructions. It would be great if you could test it again.
I have tested this item β
successfully on 0fc51d1
I have tested this successfully - thanks for the updated testing instructions @beni71 - FYI, I only got the 1st error message but the AFTER condition was spot on match, no error messages, correct Banner 1 and Banner 2 in the Alt descriptions etc etc
@beni71 - I'm going to get in trouble I fear, as I was cleaning up my test environment after testing this PR, I realized that that server is running MariaDB 10.11.14-MariaDB (and not technically exactly MySQL 8.0.4) (even though I understand MariaDB is supposed to be drop-in replacement, I'm told there are small differences), that may be why I only got 1 error message and not all 3 of them.
@exlemor Regarding MariaDB see my comment (hidden review comment) #46547 (comment) :
MySQL changed from the old Henry Spencer syntax to ICU (which uses PCRE) with version 8.0.4.
MariaDB changed from Henry Spencer syntax to PCRE with version 10.0.5. The old syntax is still supported unless the compatibility mode is switched off.
I have tested this item β
successfully on 0fc51d1
I followed the instruction and could see banner 1 and " after applying the patch
Database Type: mysql
Database Version: 9.1.0
| Status | Pending | ⇒ | Ready to Commit |
RTC
| Labels |
Added:
RTC
|
||
β Final test before merge with JBT (which is using PHP 8.4.15)
gh pr checkout 46547, both 'Mo' banners are visible and there are no deprecation notices or warnings.git switch -, repeated step 1. seen the errors
git switch - and see it working| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2025-12-14 05:54:21 |
| Closed_By | ⇒ | muhme |
Thank you @beni71 for your contribution. Thank you @brianteeman and @richard67 for support. Thank you @exlemor and @ThomasFinnern for testing.
Related issue #41183 ?