"Import Peer Reviews" button - template field names missing

Hello

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.

Thanks.

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.

I’ll report back as I make progress.

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:

    function html2text($html) {                                                                                                                                                                                 
            $html = PKPString::regexp_replace('/<[\/]?p>/', "\n", $html);
            $html = PKPString::regexp_replace('/<li>/', '&bull; ', $html);
            $html = PKPString::regexp_replace('/<\/li>/', "\n", $html);
            $html = PKPString::regexp_replace('/<br[ ]?[\/]?>/', "\n", $html);
            $html = PKPString::html2utf(strip_tags($html));
            return $html;
    }

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 problem appears to be the /e flag in the preg_replace() function which has been deprecated since PHP 7.0.0:

7.0.0 | Support for the /e modifier has been removed. Use preg_replace_callback() instead.

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):

return preg_replace($pattern . PCRE_UTF8, $replacement, $subject, $limit);

It should instead be something along the lines of:

return preg_replace_callback($pattern . PCRE_UTF8, function($matches) { foreach ($matches as $match) { return PKPString::code2utf(hexdec("\\1"), $str); } }, $subject, $limit);

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.

The fix seems to be working - I have created a pull request.

Hi @Ianvdl,

Many thanks for contributing!

Regards,
Alec Smecher
Public Knowledge Project Team

Hi @asmecher

No problem, just keeping our OJS 2 installations afloat for a little while longer until we’re ready to upgrade to OJS 3.

Thanks for the quick merge.