Follow-up from #2114563: Remove TwigReference with the help of 'without' filter replacing 'show'/'hide' functions..

Seems that |raw is still not working right after the TwigReference removal. This must have been when I was playing with the priority because now if I move the priority to a normal positive number (above the escaper). |raw works with autoescaping on.

@dawener also mentioned macros seemed to work better when this was at a higher priority.

CommentFileSizeAuthor
#10 interdiff.txt490 bytesstar-szr
#10 2221535-10.patch2.97 KBstar-szr
#7 interdiff.txt855 bytesstar-szr
#6 2221535-6.patch2.93 KBstar-szr
#6 2221535-6-fail.patch2.39 KBstar-szr
#5 2221535-5.patch3.01 KBstar-szr
#5 2221535-5-fail.patch2.47 KBstar-szr
#1 2221535-TwigNodeVisitor-priority-1.patch495 bytesjoelpittet

Comments

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
Issue tags: +Quick fix
Related issues: +#1825952: Turn on twig autoescape by default
StatusFileSize
new 2221535-TwigNodeVisitor-priority-1.patch495 bytes

Throwing this patch up to see if testbot has issues but likely needs some review to see if this will cause other issues. Fairly confident it should be fine. Let's have a look shall we?

Also, may need tests of some sort but can't think of how/what but something to show the raw and macros are working as expected.

Status: Needs review » Needs work

The last submitted patch, 1: 2221535-TwigNodeVisitor-priority-1.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Needs work » Needs review

1: 2221535-TwigNodeVisitor-priority-1.patch queued for re-testing.

star-szr’s picture

star-szr
he/him
Primary language English
commented

Test passes locally, testbot log says:

Segmentation fault
FATAL Drupal\views\Tests\Handler\AreaTest: test runner returned a non-zero error code (139).

Sent for re-test.

star-szr’s picture

star-szr
he/him
Primary language English
commented
StatusFileSize
new 2221535-5-fail.patch2.47 KB
new 2221535-5.patch3.01 KB

Here we go, this should fail/pass appropriately. Left the interdiff off because the previous patch is one line :)

I stole the drupalSetContent() trick from the twig namespace tests. Makes me want to refactor some of our theme test coverage to remove page callbacks knowing that's possible!

star-szr’s picture

star-szr
he/him
Primary language English
commented
StatusFileSize
new 2221535-6-fail.patch2.39 KB
new 2221535-6.patch2.93 KB

Fixed up a bit, will cancel the #5 patches.

star-szr’s picture

star-szr
he/him
Primary language English
commented
StatusFileSize
new interdiff.txt855 bytes

Forgot the interdiff. /me goes to bed

The last submitted patch, 5: 2221535-5.patch, failed testing.

The last submitted patch, 5: 2221535-5-fail.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
commented
StatusFileSize
new 2221535-10.patch2.97 KB
new interdiff.txt490 bytes

However might as well fix the docblock while we are here because this will conflict a bit with #2212309: Drupal\Core\Template and twig.engine docs, coding standards, and unused code cleanup anyway.

The last submitted patch, 6: 2221535-6-fail.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Awesome, #6 says it all. I'm cool with the conflict reroll in #2212309.

dawehner’s picture

dawehner
Primary language German
commented

<3

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Great catch, and with tests too! :)

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.