From ac593e928f96d8eaa0f5ae7ea5b3538ab9da2720 Mon Sep 17 00:00:00 2001 From: Ruslan Kabalin Date: Thu, 21 Feb 2019 00:00:18 +0000 Subject: [PATCH 1/2] mustache_lint: Test partials with stray tags. Stray tags are those that got strict parent requirement, e.g. you can't use
  • ...
    • without wrapping it in tag. --- tests/1-mustache_lint.bats | 21 +++++++- tests/fixtures/31-mustache_lint-partial.patch | 53 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/31-mustache_lint-partial.patch diff --git a/tests/1-mustache_lint.bats b/tests/1-mustache_lint.bats index d6ade39a..e7312eb4 100755 --- a/tests/1-mustache_lint.bats +++ b/tests/1-mustache_lint.bats @@ -76,7 +76,6 @@ setup () { # Assert result assert_failure - assert_output --partial "NPM installed validator found." assert_output --partial "Running mustache lint from $GIT_PREVIOUS_COMMIT to $GIT_COMMIT" assert_output --partial "lib/templates/linting.mustache - WARNING: HTML Validation error, line 2: End tag “p” seen, but there were open elements. (ello World

    Hello )" @@ -100,6 +99,24 @@ setup () { assert_output --partial "No mustache problems found" } +@test "mustache_lint: Partial contains elements with strict parent requirement" { + # Set up. + git_apply_fixture 31-mustache_lint-partial.patch + export GIT_PREVIOUS_COMMIT=$FIXTURE_HASH_BEFORE + export GIT_COMMIT=$FIXTURE_HASH_AFTER + + ci_run mustache_lint/mustache_lint.sh + + # Assert result + assert_success + # We should not have a validation warning about stray tags. + refute_output --partial "lib/templates/linting.mustache - WARNING: HTML Validation error, line 2: Stray start tag “td”." + refute_output --partial "lib/templates/linting.mustache - WARNING: HTML Validation error, line 2: Stray end tag “td”." + + assert_output --partial "lib/templates/linting.mustache - OK: Mustache rendered html succesfully" + assert_output --partial "No mustache problems found" +} + @test "mustache_lint: Full HTML page doesn't get embeded in body" { # Set up. git_apply_fixture 31-mustache_lint-full-html-body.patch @@ -110,7 +127,7 @@ setup () { # Assert result assert_success - # We should not have a vlidation warning about multiple 'html' tags. + # We should not have a validation warning about multiple 'html' tags. refute_output --partial 'Stray start tag “html”.' assert_output --partial "lib/templates/full-html-page.mustache - OK: Mustache rendered html succesfully" diff --git a/tests/fixtures/31-mustache_lint-partial.patch b/tests/fixtures/31-mustache_lint-partial.patch new file mode 100644 index 00000000..153a60ac --- /dev/null +++ b/tests/fixtures/31-mustache_lint-partial.patch @@ -0,0 +1,53 @@ +From 21dcf559cc7850d03729f7679e2f1ce87db5cb4c Mon Sep 17 00:00:00 2001 +From: Ruslan Kabalin +Date: Fri, 18 Jan 2019 18:26:08 +0000 +Subject: [PATCH] MDL-56504 mustache: fixture for partial problem detection + +--- + lib/templates/linting.mustache | 35 +++++++++++++++++++++++++++++++++++ + 1 file changed, 35 insertions(+) + create mode 100644 lib/templates/linting.mustache + +diff --git a/lib/templates/linting.mustache b/lib/templates/linting.mustache +new file mode 100644 +index 00000000000..dffe9239d92 +--- /dev/null ++++ b/lib/templates/linting.mustache +@@ -0,0 +1,35 @@ ++{{! ++ This file is part of Moodle - http://moodle.org/ ++ ++ Moodle is free software: you can redistribute it and/or modify ++ it under the terms of the GNU General Public License as published by ++ the Free Software Foundation, either version 3 of the License, or ++ (at your option) any later version. ++ ++ Moodle is distributed in the hope that it will be useful, ++ but WITHOUT ANY WARRANTY; without even the implied warranty of ++ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ++ GNU General Public License for more details. ++ ++ You should have received a copy of the GNU General Public License ++ along with Moodle. If not, see . ++}} ++{{! ++ @template core/linting ++ ++ A test for for mustache linting tests. ++ ++ Classes required for JS: ++ * none ++ ++ Data attributes required for JS: ++ * none ++ ++ Context variables required for this template: ++ * none ++ ++ Example context (json): ++ { ++ } ++}} ++Hello World +-- +2.11.0 From 8523227bbe5656d11abe6e3768b638b977d518c6 Mon Sep 17 00:00:00 2001 From: Ruslan Kabalin Date: Thu, 21 Feb 2019 00:02:27 +0000 Subject: [PATCH 2/2] mustache_lint: Wrap stray tags into valid parent. Attempt to fix a partial template containing elements with a strict parent requirements. --- mustache_lint/mustache_lint.php | 59 ++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 8 deletions(-) diff --git a/mustache_lint/mustache_lint.php b/mustache_lint/mustache_lint.php index a1ac17c1..f7ef9301 100644 --- a/mustache_lint/mustache_lint.php +++ b/mustache_lint/mustache_lint.php @@ -192,16 +192,59 @@ function print_message($severity, $mesage) { } /** - * Wrap the template content in a html5 wrapper and validate it + * Fix partials and wrap the template content in a html5 wrapper. + * + * There could be the case when partial template got elements that have a strict + * parent element requirement. We can wrap most obvious elements into correct parent, + * assuming the partial template is "balanced" i.e. contain equal number of start + * and end tags (though, if it is not equial, the partial template will fail + * validation anyway). This partial fixing wrapper will not be required if we + * find a way to exclude partials from linting at some point (MDL-56504). + * + * @param string $content the raw template as string. + * @return string $wrappedcontent */ -function check_html_validation($content) { - if (strpos($content, '') === false) { - // Primative detection if we have full html body, if not, wrap it. - // (This isn't bulletproof, obviously). - $wrappedcontent = "Validate\n{$content}\n"; - } else { - $wrappedcontent = $content; +function wrap_content($content) { + // Primitive detection if we have full html body, if not, we need to wrap it. + if (strpos($content, '')) { + return $content; + } + + // Mapping of child elements to their required parent elements. + $reqparent = [ + // List. + 'li' => 'ul', + // Table. + 'td' => 'tr', + 'th' => 'tr', + 'tr' => 'table', + 'thead' => 'table', + 'tbody' => 'table', + 'tfoot' => 'table', + // Definition list. + 'dd' => 'dl', + 'dt' => 'dl', + ]; + + // Determine if the first element has strict parent element requirement and wrap it if necessary. + // Iterate through, as further wrapping may be needed. + while (preg_match('@^<(' . join('|', array_keys($reqparent)) . ')[^>]*>@is', trim($content), $matches)) { + $parent = $reqparent[$matches[1]]; + $content = "<{$parent}>\n{$content}\n"; } + + // Wrap in html5 body. + $wrappedcontent = "Validate\n{$content}\n"; + return $wrappedcontent; +} + +/** + * Validate template content. + */ +function check_html_validation($content) { + // We need to wrap template content first. + $wrappedcontent = wrap_content($content); + // And then call validator. $response = validate_html($wrappedcontent); if (!$response || !isset($response->messages)) {