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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Active » Needs review
Issue tags: +Quick fix
Related issues: +#1825952: Turn on twig autoescape by default
FileSize
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
CreditAttribution: star-szr 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
CreditAttribution: star-szr 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
CreditAttribution: star-szr commented
FileSize
2221535-5-fail.patch2.47 KB
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
CreditAttribution: star-szr commented
FileSize
2221535-6-fail.patch2.39 KB
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
CreditAttribution: star-szr commented
FileSize
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
CreditAttribution: star-szr commented
FileSize
2221535-10.patch2.97 KB
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
Primary language English
Location Vancouver
CreditAttribution: joelpittet 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
CreditAttribution: dawehner commented

<3

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick 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.