Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thedavidmeister’s picture

Status: Active » Needs review
FileSize
20.59 KB

patch.

Status: Needs review » Needs work

The last submitted patch, 2089351-1.patch, failed testing.

thedavidmeister’s picture

Status: Needs work » Needs review

#1: 2089351-1.patch queued for re-testing.

thedavidmeister’s picture

Issue tags: +Novice
jibran’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Quick fix

I have searched core/includes haven't found any other occurrence so RTBC.

meba’s picture

Issue tags: +Needs security review

Adding a Security tag

meba’s picture

FileSize
20.61 KB

Looks OK to me, rerolled to apply cleanly

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

thedavidmeister’s picture

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

thedavidmeister’s picture

I don't understand why this was tagged with "needs security review".

There are still references to check_plain() in:

- core/includes/menu.inc

kirby14’s picture

There are still references to check_plain() in:

- core/includes/menu.inc

// Avoid calling check_plain again on l() function.
      if ($title_callback == 'check_plain') {
        $item['localized_options']['html'] = TRUE;
      }

Other than comments, this is the only spot, and it isn't calling check_plain();

thedavidmeister’s picture

Hmmm, I see. My bad. That's interesting, could $title_callback be expecting to use String::checkPlain() somehow?

longwave’s picture

This check was first introduced in #212409: Content Type title displays incorrectly, not sure if it is needed any more - maybe it can be safely removed?

thedavidmeister’s picture

Issue tags: -Quick fix, -Novice

Yeah, I think this needs further investigation. I'll take the novice tag off.

JiriK’s picture

Status: Needs work » Closed (fixed)

The piece of code mentioned in comment #12 was removed by #2107533: Remove {menu_router}. So I think this issue is irrelevant now.