Skip to content

Commit

Permalink
allow conversion of relative font-size from rem to percent (#24)
Browse files Browse the repository at this point in the history 
HTML answers written with Atto (the former default editor in Moodle) can sometimes have <span> 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.
  • Loading branch information
@PhilippImhof
PhilippImhof authored Oct 26, 2024
1 parent 9f3263f commit 4f50d45
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 2 deletions.
11 changes: 10 additions & 1 deletion essaydownload_form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

/**
Expand Down
6 changes: 6 additions & 0 deletions essaydownload_options.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion lang/en/quiz_essaydownload.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.';
Expand All @@ -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 <i>Atto</i> 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.<br><br>When using the plain-text summary, you might want to use a monospaced font.';
$string['fontmono'] = 'Monospaced';
Expand Down Expand Up @@ -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).<br><br>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.<br><br>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';
43 changes: 43 additions & 0 deletions report.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,11 @@ protected function generate_pdf(string $text, string $header = '', string $subhe
// rather remove it here.
$text = str_replace("\xc2\xa0", "&nbsp;", $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');
Expand Down Expand Up @@ -541,4 +546,42 @@ protected function generate_pdf(string $text, string $header = '', string $subhe
return $doc->Output('', 'S');
}

/**
* Atto sometimes adds a <span> 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"
<span[^>]*style # opening a <span> 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 <span> 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;
}

}
6 changes: 6 additions & 0 deletions tests/behat/form.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
69 changes: 69 additions & 0 deletions tests/report_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <span>bli</span> bar',
'foo <span>bli</span> bar',
],
'one span with font-size not rem' => [
'foo <span style="font-size: 15px;">bli</span> bar',
'foo <span style="font-size: 15px;">bli</span> bar',
],
'one span with font-size rem' => [
'foo <span style="font-size: 90%;">bli</span> bar',
'foo <span style="font-size: 0.9rem;">bli</span> bar',
],
'one span with font-size rem and other attributes' => [
'foo <span strangeattribute="yes" style="font-size: 90%;" anotherthing>bli</span> bar',
'foo <span strangeattribute="yes" style="font-size: 0.9rem;" anotherthing>bli</span> bar',
],
'one span with font-size rem and other properties' => [
'foo <span style="text-align: left; font-size: 90%; some-obscure-property: true;">bli</span> bar',
'foo <span style="text-align: left; font-size: 0.9rem; some-obscure-property: true;">bli</span> bar',
],
'one span with font-size rem, uppercase' => [
'foo <SPAN style="font-size: 90%;">bli</SPAN> bar',
'foo <SPAN style="font-size: 0.9rem;">bli</SPAN> bar',
],
'one span with font-size rem, single quote' => [
"foo <span style='font-size: 90%;'>bli</span> bar",
"foo <span style='font-size: 0.9rem;'>bli</span> bar",
],
'two spans with font-size rem' => [
'foo <span style="font-size: 90%;">bli</span> goo <span style="font-size: 75%;">dip</span> bar',
'foo <span style="font-size: 0.9rem;">bli</span> goo <span style="font-size: 0.75rem;">dip</span> bar',
],
'one span font-size rem with whitespace' => [
'foo <span style = "font-size: 90%;">bli</span> bar',
'foo <span style = "font-size: 0.9 rem;">bli</span> 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)
);
}

}

0 comments on commit 4f50d45

Please sign in to comment.