Drupal\views\Plugin\views\area\Result does an unnecessary XSS::adminFilter()
// Send the output.
if (!empty($total)) {
$output .= Xss::filterAdmin(str_replace(array_keys($replacements), array_values($replacements), $format));
}
// Return as render array.
return array(
'#markup' => $output,
);
Since we are returning a render array with #markup that is automatically admin filtered.
| Comment | File | Size | Author |
|---|---|---|---|
| #39 | 2569381-36.patch | 2.72 KB | lendude |
| #36 | 2569381-36.patch | 2.72 KB | init90 |
| #12 | interdiff-2569381-8-12.patch | 3.4 KB | vasi |
| #12 | 2569381-12.patch | 2.99 KB | vasi |
| #8 | unneeded_filter-2569381-2.patch | 1.68 KB | alx_benjamin |
Comments
Comment #2
alexpottComment #3
alx_benjamin commentedLet's see if this will pass testing
-------------------------------------------------
Sponsored by http://reallifedesign.co.uk/
Comment #4
alx_benjamin commentedComment #5
alx_benjamin commentedComment #6
dawehnerPerfect. As a test you probably best try to write some unit test, given that the endResult on the page will be the same.
Comment #7
alx_benjamin commentedWriting test
Comment #8
alx_benjamin commentedHope this is a right test.
Comment #9
alx_benjamin commentedComment #10
dawehnerMH actually the test should somehow use the result plugin, so what about exporting a view that has it used?
Comment #11
vasi commentedI'd like to help with this, but I'm not sure how. I guess I'd have to enable views_test_config and update it? But I'm not sure how to enable it.
Comment #12
vasi commentedOk, I figured something out, using some gross code to set things up so I could build my view. (The patch itself is clean.) I'd appreciate if someone can provide better docs on how to do this in the future!
Comment #15
deepakaryan1988Comment #16
deepakaryan1988Comment #17
duaelfrUtagging because at this state of the issue, tests has been written. Plus, reviewing that kind of issue is not a novice task as we cannot have any steps to reproduce and as we cannot do manual testing.
Comment #21
duaelfrSimple reroll on 8.4.x to see what happens.
Comment #22
duaelfrComment #23
duaelfrWrong tag, sorry
Comment #24
lendudeMassively overlaps with #2782221: Result summary Area plugin not displayed when there is no result., not that we should combine the two, but when one lands the other needs to do some refactoring to combine the tests.
Comment #25
lendudeBit of nitpicking:
We don't do @file anymore right?
can just use an {@inheritdoc}
array() => []
Comment #26
Munavijayalakshmi commentedMade changes as per the comment #25. Applying the patch, please review.
Comment #27
lendudeMissed some nitpicks the first time, sorry:
Needs a newline.
every method needs a docblock
Comment #36
init90I've found that issue as a random bug from - https://lendude.gitlab.io/bug-smash-initiative/
Comment #37
lendudeRefactored test coverage looks good, could you upload a test-only file to make sure the test still covers the bug?
Comment #38
init90Of course, here is a test only patch
Comment #39
lendude@init90 thanks! As you pointed out to me on slack, we don't have a 'fail' here because we are only removing a double clean up, so we only need the test that proves it stays working, which we do.
Ok, so this looks ready. Re-upping the patch with the fix so the right one gets queued for retesting and it is clear with one I'm RTBC'ing
Comment #40
alexpottCommitted and pushed f5fdfe3165 to 9.4.x and 16d107539f to 9.3.x. Thanks!
Backported to 9.3.x as a tested and simple bug fix.