We’re using OJS 2.4.8-4 on PHP 7. I’m investigating an issue where the “Import Peer Reviews” button fills in the reviewers and their comments, but not the names of the fields. Something similar to the following:
Reviewer A:
: A comment
: Another comment
The text prior to the colon is not filled in for any of the fields.
It does this for all the reviewers. The form is set up to show these fields to the author. I can also see the questions in the review_form_element_settings database table. The journal language corresponds to the locale set in that database table as well (en_US). I don’t see any errors in the web server logs.
Where can I start looking for the problem? Is there a sub-template for the section that is generated when the button is pressed? I looked at setReviewerRecommendation() in classes/submission/sectionEditor/SectionEditorAction.inc.php:720, where the documentation states “Also concatenates the reviewer and editor comments from Peer Review and adds them to Editor Review.”, but I can’t tell where that concatenation actually happens.
For anyone else having this problem, I have since found the section responsible for inserting the reviews (I was looking in the wrong part of the file yesterday). The section starts at classes/submission/sectionEditor/SectionEditorAction.inc.php:2068.
At classes/submission/sectionEditor/SectionEditorAction.inc.php:2100 there is a line with a call to PKPString::html2text: $body .= PKPString::html2text($reviewFormElement->getLocalizedQuestion()) . ": \n";
The return value of $reviewFormElement->getLocalizedQuestion() contains the correct value as stored in the database at this point. It is then sent to html2text() at lib/pkp/classes/core/PKPString.inc.php:527, which looks as follows:
At every point in this function the values are retained, apart from the html2utf() call (it works up to and including strip_tags()). The html2utf() call at lib/pkp/classes/core/PKPString.inc.php:717 looks as follows:
function html2utf($str) {
// convert named entities to numeric entities
$str = strtr($str, PKPString::getHTMLEntities());
// use PCRE-aware replace function to replace numeric entities
$str = PKPString::regexp_replace('~&#x([0-9a-f]+);~ei', 'PKPString::code2utf(hexdec("\\1"))', $str);
$str = PKPString::regexp_replace('~&#([0-9]+);~e', 'PKPString::code2utf(\\1)', $str);
return $str;
}
The values are still retained after $str = strtr($str, PKPString::getHTMLEntities());, but the $str variable is empty after the first regexp_replace() call.
The investigation continues to see what the root cause is - but given that it’s lead so far down the stack there might be other pages also affected by this.
Edit: The last change to these lines was the replacement for String → PKPString 5 months ago. Prior to that the last change to this particular line appears to have been 11 years ago.
The line $str = PKPString::regexp_replace('~&#x([0-9a-f]+);~ei', 'PKPString::code2utf(hexdec("\\1"))', $str);
should therefore be replaced with: $str = PKPString::regexp_replace('~&#x([0-9a-f]+);~i', 'PKPString::code2utf(hexdec("\\1"))', $str);
And regexp_replace() should call the preg_replace_callback() function instead of the preg_replace() function. The code is currently (at line 353):
I just need to check how to evaluate the passed function inside the loop with PHP.
A similar change to remove the /e flag from the other regexp_replace() call in the same function is also required. I will test this change and create a pull request for this issue.