The current $request_path generation relies on global $base_url.
This approach breaks with the OOP and DI pattern of Drupal.

Current code:

global $base_url;
$request_path = urlencode($base_url . $this->currentPath->getPath());

// ...

$build['open_readspeaker_block'] = [
      '#theme' => 'open_readspeaker_ui',
      '#request_path' => $request_path,
];

We could simply replace that with Url::fromRoute()

$build['open_readspeaker_block'] = [
      '#theme' => 'open_readspeaker_ui',
      '#request_path' => urlencode(Url::fromRoute('<current>', [], ['absolute' => TRUE])->toString()),
];

In addition to the new code we could remove the DI of the $container->get('path.current') service.

the result is the same as before, but without any "magic" global variable.

Any thoughts?

Comments

sunlix created an issue. See original summary.

sunlix’s picture

StatusFileSize
new3.61 KB
sunlix’s picture

Title: OpenReadspeakerBlock: replace $request_path generation with Url::fromRoute() » OpenReadspeakerBlock: refactor $request_path generation along with template improvements
sunlix’s picture

StatusFileSize
new5.74 KB

This patch removes unnecessary current_path DI along with template improvements for the ReadSpeaker endpoint query parameter generation.
In core, optional parameters will be filtered if they are not set.

  • sunlix authored ef4d253 on 8.x-1.x
    Issue #3146505 by sunlix: OpenReadspeakerBlock: refactor $request_path...
sunlix’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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