View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 19685 | Bug reports | Other | public | 2024-08-08 14:08 | 2024-11-19 18:14 |
| Reporter | tibor.pacalat | Assigned To | gabrieljenik | ||
| Priority | none | Severity | minor | ||
| Status | closed | Resolution | fixed | ||
| Product Version | 6.6.x | ||||
| Summary | 19685: php8.3 compatibility - LS Core | ||||
| Description | Main ticket https://bugs.limesurvey.org/view.php?id=19632 LS Core | ||||
| Steps To Reproduce | Steps to reproduce(Replace this text with detailed step-by-step instructions on how to reproduce the issue) Expected result(Write here what you expected to happen) Actual result(Write here what happened instead) | ||||
| Tags | No tags attached. | ||||
| Bug heat | 14 | ||||
| Complete LimeSurvey version number (& build) | 6.6.1+240806 | ||||
| I will donate to the project if issue is resolved | No | ||||
| Browser | |||||
| Database type & version | . | ||||
| Server OS (if known) | |||||
| Webserver software & version (if known) | |||||
| PHP Version | . | ||||
| related to | 19747 | new | Sometimes the theme configuration hirearchy loading is not correct | |
| child of | 19632 | closed | gabrieljenik | Make Limesurvey compatible with php8.3 |
|
I have scanned the log with the Rector tool, which is I think more precise. rector.log (10,620 bytes)
application/controllers
1 file with changes
===================
1) application/controllers/ThemeOptionsController.php:361
---------- begin diff ----------
@@ @@
*
* @return void
*/
- public function actionUpdateSurveyGroup(int $id = null, int $gsid, $l = null)
+ public function actionUpdateSurveyGroup(int $gsid, int $id = null, $l = null)
{
if (!Permission::model()->hasGlobalPermission('templates', 'update')) {
if (empty($gsid)) {
----------- end diff -----------
Applied rules:
* OptionalParametersAfterRequiredRector
-- NOTE: This is a false positive. This doesn't fail when the parameter is typed and the default value is null.
[OK] 1 file would have been changed (dry-run) by Rector
application/core
[WARNING] The following file was skipped as starting with short open tag. Migrate to long open PHP tag first:
application/core/plugins/TwoFactorAdminLogin/views/_partial/create.php
2 files with changes
====================
1) application/core/plugins/AzureOAuthSMTP/vendor/paragonie/random_compat/other/build_phar.php:7
---------- begin diff ----------
@@ @@
}
$phar = new Phar(
$dist.'/random_compat.phar',
- FilesystemIterator::CURRENT_AS_FILEINFO | \FilesystemIterator::KEY_AS_FILENAME,
+ FilesystemIterator::CURRENT_AS_FILEINFO | \FilesystemIterator::KEY_AS_FILENAME | \FilesystemIterator::SKIP_DOTS,
'random_compat.phar'
);
rename(
----------- end diff -----------
Applied rules:
* FilesystemIteratorSkipDotsRector
-- NOTE: This is a plugin's vendor folder. Updating the dependencies will probably be enough. Anyway, if this fails, it fails in 8.2 already: https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#filesystemiteratorskipdotsrector
2) application/core/plugins/TwoFactorAdminLogin/helper/phpqrcode.php:1242
---------- begin diff ----------
@@ @@
case QR_MODE_NUM: $bits = QRinput::estimateBitsModeNum($this->size); break;
case QR_MODE_AN: $bits = QRinput::estimateBitsModeAn($this->size); break;
case QR_MODE_8: $bits = QRinput::estimateBitsMode8($this->size); break;
- case QR_MODE_KANJI: $bits = QRinput::estimateBitsModeKanji($this->size);break;
+ case QR_MODE_KANJI: $bits = (new QRinput())->estimateBitsModeKanji($this->size);break;
case QR_MODE_STRUCTURE: return STRUCTURE_HEADER_BITS;
default:
return 0;
----------- end diff -----------
Applied rules:
* StaticCallOnNonStaticToInstanceCallRector
-- NOTE: This is supposed to be failing since 8.0, but I'm not sure if it's ever called. Anyway, we should probably stop using this library. It's been deprecated for a while according to the project's README (https://github.com/t0k4rt/phpqrcode?tab=readme-ov-file).
[OK] 2 files would have been changed (dry-run) by Rector
application/helpers
6 files with changes
====================
1) application/helpers/Zend/Http/UserAgent/Features/Adapter/TeraWurfl.php:88
---------- begin diff ----------
@@ @@
if (!is_array($group)) {
continue;
}
- while (list ($key, $value) = each($group)) {
+ foreach ($group as $key => $value) {
if (is_bool($value)) {
// to have the same type than the official WURFL API
$features[$key] = ($value ? 'true' : 'false');
----------- end diff -----------
Applied rules:
* WhileEachToForeachRector
-- NOTE: This should be failing since 7.2, but we are not using that class.
2) application/helpers/userstatistics_helper.php:2620
---------- begin diff ----------
@@ @@
}
// Creating the first worksheet
- $this->sheet = $this->workbook->addWorksheet(utf8_decode('results-survey' . $surveyid));
+ $this->sheet = $this->workbook->addWorksheet(mb_convert_encoding('results-survey' . $surveyid, 'ISO-8859-1'));
$this->xlsPercents = &$this->workbook->addFormat();
$this->xlsPercents->setNumFormat('0.00%');
$this->formatBold = &$this->workbook->addFormat(array('Bold' => 1));
----------- end diff -----------
Applied rules:
* Utf8DecodeEncodeToMbConvertEncodingRector
-- NOTE: utf8_decode has been deprecated in 8.2. We should check if it's failing.
3) application/helpers/admin/import_helper.php:1635
---------- begin diff ----------
@@ @@
}
// #14646: fix utf8 encoding issue
if (!mb_detect_encoding($insertdata['group_name'], 'UTF-8', true)) {
- $insertdata['group_name'] = utf8_encode($insertdata['group_name']);
+ $insertdata['group_name'] = mb_convert_encoding($insertdata['group_name'], 'UTF-8', 'ISO-8859-1');
}
// Insert the new group
$oQuestionGroupL10n = new QuestionGroupL10n();
----------- end diff -----------
Applied rules:
* Utf8DecodeEncodeToMbConvertEncodingRector
-- NOTE: utf8_decode has been deprecated in 8.2. We should check if it's failing.
4) application/helpers/admin/statistics_helper.php:3855
---------- begin diff ----------
@@ @@
}
// Creating the first worksheet
- $this->sheet = $this->workbook->addWorksheet(utf8_decode('results-survey' . $surveyid));
+ $this->sheet = $this->workbook->addWorksheet(mb_convert_encoding('results-survey' . $surveyid, 'ISO-8859-1'));
$this->xlsPercents = $this->workbook->addFormat();
$this->xlsPercents->setNumFormat('0.00%');
$this->formatBold = $this->workbook->addFormat(array('Bold' => 1));
----------- end diff -----------
Applied rules:
* Utf8DecodeEncodeToMbConvertEncodingRector
-- NOTE: utf8_decode has been deprecated in 8.2. We should check if it's failing.
5) application/helpers/ldap_helper.php:27
---------- begin diff ----------
@@ @@
} else {
$ds = false;
if ($ldap_server[$server_id]['protoversion'] == 'ldapv3' && $ldap_server[$server_id]['encrypt'] != 'ldaps') {
- $ds = ldap_connect($ldap_server[$server_id]['server'], $ldap_server[$server_id]['port']);
+ $ds = ldap_connect("{$ldap_server[$server_id]['server']}:{$ldap_server[$server_id]['port']}");
ldap_set_option($ds, LDAP_OPT_PROTOCOL_VERSION, 3);
if (!$ldap_server[$server_id]['referrals']) {
@@ @@
if ($ldap_server[$server_id]['encrypt'] == 'ldaps') {
$ds = ldap_connect("ldaps://" . $ldap_server[$server_id]['server'] . ':' . $ldap_server[$server_id]['port']);
} else {
- $ds = ldap_connect($ldap_server[$server_id]['server'], $ldap_server[$server_id]['port']);
+ $ds = ldap_connect("{$ldap_server[$server_id]['server']}:{$ldap_server[$server_id]['port']}");
}
if (!$ldap_server[$server_id]['referrals']) {
----------- end diff -----------
Applied rules:
* CombineHostPortLdapUriRector
-- NOTE: This needs to be fixed for 8.3, as ldap_connect no longer supports the second parameter.
6) application/helpers/qanda_helper.php:3535
---------- begin diff ----------
@@ @@
), true);
} else {
$answer = doRender('/survey/questions/answer/arrays/array/dropdown/empty', [], true);
- $inputnames = '';
+ $inputnames = [];
}
return array($answer, $inputnames);
}
----------- end diff -----------
Applied rules:
* AssignArrayToStringRector
-- NOTE: It probably wouldn't hurt, but it looks like a false positive. What's been deprecated in 7.1 is the assignment as array on an empty string, which is not happening here. (See: https://www.php.net/manual/en/migration71.incompatible.php#migration71.incompatible.empty-string-modifcation-by-character)
[OK] 6 files would have been changed (dry-run) by Rector
application/models
1 file with changes
===================
1) application/models/SurveyLink.php:87
---------- begin diff ----------
@@ @@
$this->deleteLinksBySurvey($iSurveyId);
$tableName = "{{tokens_" . $iSurveyId . "}}";
$dateCreated = date('Y-m-d H:i:s', time());
- $query = "INSERT INTO " . SurveyLink::tableName() . " (participant_id, token_id, survey_id, date_created) SELECT participant_id, tid, '" . $iSurveyId . "', '" . $dateCreated . "' FROM " . $tableName . " WHERE participant_id IS NOT NULL";
+ $query = "INSERT INTO " . (new SurveyLink())->tableName() . " (participant_id, token_id, survey_id, date_created) SELECT participant_id, tid, '" . $iSurveyId . "', '" . $dateCreated . "' FROM " . $tableName . " WHERE participant_id IS NOT NULL";
return Yii::app()->db->createCommand($query)
->query();
}
@@ @@
*/
public function deleteTokenLink($aTokenIds, $surveyId)
{
- $query = "DELETE FROM " . SurveyLink::tableName()
+ $query = "DELETE FROM " . (new SurveyLink())->tableName()
. " WHERE token_id IN (" . implode(", ", $aTokenIds) . ") AND survey_id=:survey_id";
return Yii::app()->db->createCommand($query)
->bindParam(":survey_id", $surveyId)
@@ @@
*/
public function deleteLinksBySurvey($surveyId)
{
- $query = "DELETE FROM " . SurveyLink::tableName() . " WHERE survey_id = :survey_id";
+ $query = "DELETE FROM " . (new SurveyLink())->tableName() . " WHERE survey_id = :survey_id";
return Yii::app()->db->createCommand($query)
->bindParam(":survey_id", $surveyId)
->query();
----------- end diff -----------
Applied rules:
* StaticCallOnNonStaticToInstanceCallRector
[OK] 1 file would have been changed (dry-run) by Rector
-- NOTE: This is supposed to be failing since 8.0. I'm not sure why it's not failing. In fact, tableName() is also used like that in other places:
- application/models/Answer.php
- application/models/Question.php
- application/models/TokenDynamic.php
|
|
|
The library used to generate statistics in Excel does not work in PHP 8.3. GitHub - pear/Spreadsheet_Excel_Writer (https://github.com/pear/Spreadsheet_Excel_Writer): Allows writing of Excel spreadsheets. Since 2002. We are already on the latest version. The project is not archived, but it does not have any updates either. I don't know if it is worth sending a PR. In 8.2 it is already working badly. |
|
|
We already have a different Excel library in use for export responses, don't we? Can we use that one? |
|
|
We use PHPOffice/PhpSpreadsheet in other projects. Lime uses two. Alt A: Use mk-j/php_xlsxwriter on statistics Probably the best would be to handle that on another ticket |
|
|
https://github.com/LimeSurvey/LimeSurvey/pull/3960 While testing/reviewing we found a bug related to this piece of code: The bug seems to be existent in master. |
|
|
@gabrieljenik what is the bug that you found? You can create a separate ticket for it, but please put a note here with a link to that bugfix PR. |
|
mk-j/php_xlsxwriter is really a great tool , and https://github.com/mk-j/PHP_XLSXWriter/commits?author=c-schmitz + https://github.com/mk-j/PHP_XLSXWriter/pulls?q=is%3Apr+author%3A%40me+is%3Aclosed The commit seems related to core here ? Not related to XLS ? |
|
|
The commit fox other issues than core (due to vendor update) |
|
Yes but is not as much as updated as the PHPOffice/PhpSpreadsheet.
It is related to a library. We should use another ticket. |
|
@tibor.pacalat, I have created 19747 |
|
|
New PR for master: https://github.com/LimeSurvey/LimeSurvey/pull/3962 |
|
|
Why you didn't replace And still have some broken test too. |
|
|
Why is the new PR a draft? |
|
|
@gabrieljenik code-check tests were failing on your PR and then I merged master into your branch and now functional tests are failing. Can you please look into this and make it green? Thanks in advance https://github.com/LimeSurvey/LimeSurvey/actions/runs/11051940595/job/30702935773 |
|
|
Check now. I have just re ran the jobs. Green now. |
|
|
I have tested and merged this in. Hopefully everything goes well :) |
|
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2024-08-08 14:08 | tibor.pacalat | New Issue | |
| 2024-08-14 20:34 | gabrieljenik | Assigned To | => gabrieljenik |
| 2024-08-14 20:34 | gabrieljenik | Status | new => assigned |
| 2024-08-20 00:18 | gabrieljenik | Note Added: 80783 | |
| 2024-08-20 00:18 | gabrieljenik | File Added: rector.log | |
| 2024-08-20 00:18 | gabrieljenik | Bug heat | 0 => 2 |
| 2024-09-09 15:49 | gabrieljenik | Note Added: 80932 | |
| 2024-09-11 12:19 | c_schmitz | Note Added: 80954 | |
| 2024-09-11 12:19 | c_schmitz | Bug heat | 2 => 4 |
| 2024-09-11 16:36 | gabrieljenik | Note Added: 80966 | |
| 2024-09-11 16:36 | gabrieljenik | File Added: image.png | |
| 2024-09-11 16:50 | gabrieljenik | Note Edited: 80966 | |
| 2024-09-11 16:54 | gabrieljenik | Assigned To | gabrieljenik => DenisChenu |
| 2024-09-11 16:54 | gabrieljenik | Status | assigned => ready for code review |
| 2024-09-11 16:54 | gabrieljenik | Note Added: 80967 | |
| 2024-09-11 16:57 | gabrieljenik | Note Edited: 80967 | |
| 2024-09-12 10:25 | tibor.pacalat | Note Added: 80978 | |
| 2024-09-12 10:25 | tibor.pacalat | Bug heat | 4 => 6 |
| 2024-09-12 11:15 | DenisChenu | Note Added: 80981 | |
| 2024-09-12 11:15 | DenisChenu | Bug heat | 6 => 8 |
| 2024-09-12 11:17 | DenisChenu | Relationship added | child of 19632 |
| 2024-09-12 11:32 | DenisChenu | Assigned To | DenisChenu => tibor.pacalat |
| 2024-09-12 11:32 | DenisChenu | Status | ready for code review => ready for testing |
| 2024-09-12 11:32 | DenisChenu | Note Added: 80987 | |
| 2024-09-12 14:46 | guest | Bug heat | 8 => 14 |
| 2024-09-12 15:38 | gabrieljenik | Note Added: 80996 | |
| 2024-09-12 15:44 | gabrieljenik | Relationship added | related to 19747 |
| 2024-09-12 15:45 | gabrieljenik | Note Added: 81000 | |
| 2024-09-13 22:30 | gabrieljenik | Note Added: 81025 | |
| 2024-09-13 22:31 | gabrieljenik | Assigned To | tibor.pacalat => DenisChenu |
| 2024-09-13 22:31 | gabrieljenik | Status | ready for testing => ready for code review |
| 2024-09-16 11:33 | DenisChenu | Assigned To | DenisChenu => gabrieljenik |
| 2024-09-16 11:33 | DenisChenu | Status | ready for code review => in code review |
| 2024-09-17 17:15 | gabrieljenik | Assigned To | gabrieljenik => DenisChenu |
| 2024-09-18 14:58 | DenisChenu | Note Added: 81038 | |
| 2024-09-18 14:58 | DenisChenu | Assigned To | DenisChenu => gabrieljenik |
| 2024-09-18 14:59 | DenisChenu | Note Edited: 81038 | |
| 2024-09-18 15:36 | tibor.pacalat | Note Added: 81041 | |
| 2024-09-26 14:27 | tibor.pacalat | Note Added: 81116 | |
| 2024-09-27 22:58 | gabrieljenik | Note Added: 81126 | |
| 2024-09-30 14:05 | tibor.pacalat | Note Added: 81129 | |
| 2024-10-02 16:27 | tibor.pacalat | Status | in code review => resolved |
| 2024-10-02 16:27 | tibor.pacalat | Resolution | open => fixed |
| 2024-11-19 18:14 | c_schmitz | Status | resolved => closed |