Problem/Motivation

Discovered at #1538118: Update status does not verify the identity or authenticity of the release history URL...

\Drupal\update\Form\UpdateManagerUpdate::buildForm() is currently doing this:

    $last_markup = [
      '#theme' => 'update_last_check',
      '#last' => $this->state->get('update.last_check', 0),
    ];
    $form['last_check'] = [
      '#markup' => \Drupal::service('renderer')->render($last_markup),
    ];

A) Not properly injected renderer service.

However, more to the point:

B) Pre-rendering this is silly -- the form is a giant render array, already.

Steps to reproduce

Visually inspect core/modules/update/src/Form/UpdateManagerUpdate.php and search for 'renderer'.

Proposed resolution

Don't pre-render, and let $form['last_check'] remain a render array until the form itself is rendered.

Remaining tasks

  1. Do it.
  2. Reviews / refinements.
  3. RTBC.
  4. Commit.

User interface changes

Nope.

API changes

None.

Data model changes

N/A

Release notes snippet

Nope.

Comments

dww created an issue. See original summary.

dww’s picture

Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new858 bytes
dww’s picture

Issue summary: View changes

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

tedbow’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

@dww good catch. I would RTBC but noticed that 'Last checked" doesn't appear anywhere in core/modules/update/tests/src should assert that this actually gets rendered?(though I am sure this shouldn't break anything)

I think if we just add the check to \Drupal\Tests\update\Functional\UpdateTestBase::standardTests() this should work and be called many places.

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new1.5 KB

How's this, @tedbow?

phenaproxima’s picture

StatusFileSize
new532 bytes

Oops, forgot the interdiff.

tedbow’s picture

Status: Needs review » Reviewed & tested by the community

Looks good! Thanks @dww and @phenaproxima!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 6a0d497 and pushed to 9.3.x. Thanks!

  • alexpott committed 6a0d497 on 9.3.x
    Issue #3180382 by phenaproxima, dww, tedbow: UpdateManagerUpdate doesn't...

Status: Fixed » Closed (fixed)

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