From 4f50d4504daec0e43e8dd98d604f5bbce85716c4 Mon Sep 17 00:00:00 2001 From: Philipp Imhof <52650214+PhilippImhof@users.noreply.github.com> Date: Sat, 26 Oct 2024 15:27:01 +0200 Subject: [PATCH] allow conversion of relative font-size from rem to percent (#24) HTML answers written with Atto (the former default editor in Moodle) can sometimes have tags around chunks of text that set the font-size to a relative size like 0.9375rem, see https://tracker.moodle.org/browse/MDL-67360. This leads to unreadably small text in the PDF. Users can now choose whether those font sizes should be converted to percent values which work fine with TCPDF. The function can be disabled for cases where it would not work properly or cause undesired side effects. --- essaydownload_form.php | 11 +++++- essaydownload_options.php | 6 +++ lang/en/quiz_essaydownload.php | 4 +- report.php | 43 +++++++++++++++++++++ tests/behat/form.feature | 6 +++ tests/report_test.php | 69 ++++++++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 2 deletions(-) diff --git a/essaydownload_form.php b/essaydownload_form.php index 0834521..5f2fd1a 100644 --- a/essaydownload_form.php +++ b/essaydownload_form.php @@ -130,10 +130,19 @@ protected function standard_preference_fields(MoodleQuickForm $mform) { $mform->addElement( 'advcheckbox', 'shortennames', - get_string('compatibility', 'quiz_essaydownload'), + get_string('troubleshooting', 'quiz_essaydownload'), get_string('shortennames', 'quiz_essaydownload') ); $mform->addHelpButton('shortennames', 'shortennames', 'quiz_essaydownload'); + $mform->addElement( + 'advcheckbox', + 'fixremfontsize', + '', + get_string('fixremfontsize', 'quiz_essaydownload') + ); + $mform->disabledIf('fixremfontsize', 'fileformat', 'neq', 'pdf'); + $mform->disabledIf('fixremfontsize', 'source', 'neq', 'html'); + $mform->addHelpButton('fixremfontsize', 'fixremfontsize', 'quiz_essaydownload'); } /** diff --git a/essaydownload_options.php b/essaydownload_options.php index f37f485..3061aa1 100644 --- a/essaydownload_options.php +++ b/essaydownload_options.php @@ -49,6 +49,9 @@ class quiz_essaydownload_options extends quiz_essaydownload_options_parent_class /** @var string file format TXT or PDF */ public $fileformat = 'pdf'; + /** @var bool whether to try to work around Atto bug MDL-67360 */ + public $fixremfontsize = true; + /** @var string base font family for PDF export */ public $font = 'sansserif'; @@ -111,6 +114,7 @@ public function get_initial_form_data() { $toform->attachments = $this->attachments; $toform->fileformat = $this->fileformat; + $toform->fixremfontsize = $this->fixremfontsize; $toform->font = $this->font; $toform->fontsize = $this->fontsize; $toform->groupby = $this->groupby; @@ -136,6 +140,7 @@ public function get_initial_form_data() { public function setup_from_form_data($fromform): void { $this->attachments = $fromform->attachments; $this->fileformat = $fromform->fileformat; + $this->fixremfontsize = $fromform->fixremfontsize; $this->font = $fromform->font ?? ''; $this->fontsize = $fromform->fontsize ?? ''; $this->groupby = $fromform->groupby; @@ -157,6 +162,7 @@ public function setup_from_form_data($fromform): void { public function setup_from_params() { $this->attachments = optional_param('attachments', $this->attachments, PARAM_BOOL); $this->fileformat = optional_param('fileformat', $this->fileformat, PARAM_ALPHA); + $this->fixremfontsize = optional_param('fixremfontsize', $this->fixremfontsize, PARAM_BOOL); $this->font = optional_param('font', $this->font, PARAM_ALPHA); $this->fontsize = optional_param('fontsize', $this->fontsize, PARAM_INT); $this->groupby = optional_param('groupby', $this->groupby, PARAM_ALPHA); diff --git a/lang/en/quiz_essaydownload.php b/lang/en/quiz_essaydownload.php index b220210..c092902 100644 --- a/lang/en/quiz_essaydownload.php +++ b/lang/en/quiz_essaydownload.php @@ -26,7 +26,6 @@ $string['attachments'] = 'Attachments'; $string['byattempt'] = 'Attempt'; $string['byquestion'] = 'Question'; -$string['compatibility'] = 'Compatibility setting'; $string['errorfilename'] = 'error-{$a}.txt'; $string['errorfontsize'] = 'Font size should be an integer between 6 and 50.'; $string['errormargin'] = 'All page margins must be integers between 0 and 80.'; @@ -37,6 +36,8 @@ $string['fileformatpdf'] = 'Portable Document Format (PDF)'; $string['fileformattxt'] = 'Plain-text (TXT)'; $string['firstlast'] = 'First name - Last name'; +$string['fixremfontsize'] = 'Avoid chunks of unreadably small text.'; +$string['fixremfontsize_help'] = 'Sometimes, Moodle\'s HTML editor Atto might add unwanted font size commands that will make the text unreadably small in the PDF. This setting will work around that bug.'; $string['font'] = 'Font'; $string['font_help'] = 'Note that when using the original HTML formatted text, the actual font may still be different, according to the formatting.

When using the plain-text summary, you might want to use a monospaced font.'; $string['fontmono'] = 'Monospaced'; @@ -75,3 +76,4 @@ $string['source_help'] = 'If the question text and/or the student\'s response is written in HTML format, Moodle will automatically generate a plain-text summary of the formatted text. That summary will have all HTML tags removed and some basic formatting applied (e. g. headings and bold font transformed to ALL CAPS).

When generating PDF files, you can choose whether you want to use that summary or the original question text / student answer with its formatting. If you choose the summary, you should probably use a monospaced font as well.

Note that you cannot use the formatted original text when generating TXT files. Also note that the setting will not have any effect if the student was asked to write their answer in non-HTML format, e. g. plain-text.'; $string['sourceoriginal'] = 'Original HTML formatted text'; $string['sourcesummary'] = 'Plain-text summary'; +$string['troubleshooting'] = 'Troubleshooting'; diff --git a/report.php b/report.php index 18ad71e..e4c9cb2 100644 --- a/report.php +++ b/report.php @@ -500,6 +500,11 @@ protected function generate_pdf(string $text, string $header = '', string $subhe // rather remove it here. $text = str_replace("\xc2\xa0", " ", $text); + // If using the original text, work around a bug with Atto, see MDL-82753 and MDL-67630. + if ($this->options->source === 'html') { + $text = $this->workaround_atto_font_size_issue($text); + } + $doc = new pdf('P', 'mm', $this->options->pageformat); $doc->SetCreator('quiz_essaydownload plugin for Moodle LMS'); @@ -541,4 +546,42 @@ protected function generate_pdf(string $text, string $header = '', string $subhe return $doc->Output('', 'S'); } + /** + * Atto sometimes adds a tag setting the font size to some rem value, e. g. 0.9375rem. This + * will cause the text to be extremely small in the resulting PDF. We try our best to convert those + * rem sizes into the appropriate point size, based on the general font size. + * + * @param string $input the HTML content + * @return string + */ + public function workaround_atto_font_size_issue(string $input): string { + $pattern = '| + ( # capturing group #1 for the "prefix" + ]*style # opening a tag, any stuff before the style attribute + \s*=\s* # equal sign may be surrounded by whitespace + ([\'"]) # opening quote may be single or double, capture #2 for closing quote + [^\2]*font-size # arbitrary content before the font-size property + \s*:\s* # colon may be surrounded by whitespace + ) # end of capturing group for the "prefix" + ([.0-9]+) # capture the numeric value, group #3 + \s*rem # only match for unit rem, other units do not seem to cause trouble + ( # capturing group #4 for the "suffix" + [^\2]* # any other stuff except the opening quote in the style attribute after the font-size + \2 # the closing quote of the style attribute + [^>]*> # possibly other attributes and stuff plus the end of the tag + ) # end of capturing group for the "suffix" + |xiU'; + + $res = preg_replace_callback( + $pattern, + function ($matches) { + $newsize = round(floatval($matches[3]) * 100); + return $matches[1] . $newsize . '%' . $matches[4]; + }, + $input + ); + + return $res; + } + } diff --git a/tests/behat/form.feature b/tests/behat/form.feature index 2310b3e..e56b4c6 100644 --- a/tests/behat/form.feature +++ b/tests/behat/form.feature @@ -53,6 +53,7 @@ Feature: Validation and display of the form When I am on the "Quiz 1" "quiz_essaydownload > essaydownload report" page logged in as "teacher1" When I set the field "fileformat" to "txt" Then the "source" "select" should be disabled + And the "fixremfontsize" "field" should be disabled And the "page" "select" should be disabled And the "marginleft" "field" should be disabled And the "marginright" "field" should be disabled @@ -61,3 +62,8 @@ Feature: Validation and display of the form And the "linespacing" "select" should be disabled And the "font" "select" should be disabled And the "fontsize" "field" should be disabled + + Scenario: Font size workaround should be disabled if source is summary + When I am on the "Quiz 1" "quiz_essaydownload > essaydownload report" page logged in as "teacher1" + When I set the field "source" to "plain" + Then the "fixremfontsize" "field" should be disabled diff --git a/tests/report_test.php b/tests/report_test.php index 71ebc97..2a57e99 100644 --- a/tests/report_test.php +++ b/tests/report_test.php @@ -1017,4 +1017,73 @@ public function test_txt_when_input_is_html(): void { } } + /** + * Provide data to test the Atto workaround. First line is the expected output, + * second line is the input. + * + * @return array + */ + public static function provide_texts_with_rem_font_span(): array { + return [ + 'nothing' => [ + 'foo bar', + 'foo bar', + ], + 'one span not font-size' => [ + 'foo bli bar', + 'foo bli bar', + ], + 'one span with font-size not rem' => [ + 'foo bli bar', + 'foo bli bar', + ], + 'one span with font-size rem' => [ + 'foo bli bar', + 'foo bli bar', + ], + 'one span with font-size rem and other attributes' => [ + 'foo bli bar', + 'foo bli bar', + ], + 'one span with font-size rem and other properties' => [ + 'foo bli bar', + 'foo bli bar', + ], + 'one span with font-size rem, uppercase' => [ + 'foo bli bar', + 'foo bli bar', + ], + 'one span with font-size rem, single quote' => [ + "foo bli bar", + "foo bli bar", + ], + 'two spans with font-size rem' => [ + 'foo bli goo dip bar', + 'foo bli goo dip bar', + ], + 'one span font-size rem with whitespace' => [ + 'foo bli bar', + 'foo bli bar', + ], + ]; + } + + /** + * Test relative font-size conversion from rem to percent. + * + * @dataProvider provide_texts_with_rem_font_span + * + * @param string $expected expected output after conversion + * @param string $input input + * @return void + */ + public function test_workaround_atto_font_size_issue(string $expected, string $input): void { + $report = new quiz_essaydownload_report(); + + self::assertEquals( + $expected, + $report->workaround_atto_font_size_issue($input) + ); + } + }