Support from Acquia helps fund testing for Drupal Acquia logo

Comments

droplet created an issue. See original summary.

Wim Leers’s picture

Issue tags: +Needs backport to D7

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

aerozeppelin’s picture

Initial patch.

Status: Needs review » Needs work

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

cilefen’s picture

Title: drupalSettings.path.currentPath in frontpage should returns "" » drupalSettings.path.currentPath in frontpage should return ""
Issue summary: View changes
cilefen’s picture

Title: drupalSettings.path.currentPath in frontpage should return "" » drupalSettings.path.currentPath on frontpage should return ""
dawehner’s picture

+++ b/core/modules/system/system.module
@@ -695,7 +695,7 @@ function system_js_settings_alter(&$settings, AttachedAssetsInterface $assets) {
+    'currentPath' => \Drupal::service('path.matcher')->isFrontPage() ? '' : $current_path,
...
     'isFront' => \Drupal::service('path.matcher')->isFrontPage(),

Note: We could store the path.matcher in a variable

aerozeppelin’s picture

Status: Needs work » Needs review
FileSize
1.13 KB
2.47 KB

Changes as per #9.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

This looks great for me

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2707933-10.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

sudhanshug’s picture

The patch no longer could be applied to 8.4.x
I re-rolled the patch to apply cleanly to the latest version.

sudhanshug’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 14: currentpath-2707933-14.patch, failed testing.

sudhanshug’s picture

Status: Needs work » Needs review

In #14, it shows that the patch passes the tests. But in #16, I don't how it is saying that the patch failed testing.
It may be jenkins bug.
Changing the status back to 'needs review'

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

raphaeltbm’s picture

I don't agree at all with the proposed solution here.
Plus there is a high risk of side-effects with the current solution (search for "curentPath" to see the current usage in core and contribs).

currentPath is designated to give the current internal path, there is nothing wrong with the returned value on a frontpage context or whatever the context.
If you need a friendly URL of the current page for tracking or SEO or other purpose, you should add a new key in $path_settings and do something like:

$is_front_page = \Drupal::service('path.matcher')->isFrontPage();
$current_route = $is_front_page ? '<front>' : '<current>';
// ...
'currentUrl' => Url::fromRoute($current_route)->toString(),
'isFront' => $is_front_page,

Or do that in a my_module_page_attachments_alter() via $attachments['#attached']['drupalSettings']['my-module'].

And you can always use drupalSettings.path.isFront for specific scenarios on the frontpage.

I hide the current patches proposed as they could mislead some people. Feel free to open the issue again if you really want to add a new key in drupalSettings.path for the core or if you think that i'm wrong.

droplet’s picture

Status: Closed (works as designed) » Needs review

Hmm, you're right...

We have to add something like `slug` then and rename currentUrl to internalPath (matching D8's PHP code$url->getInternalPath())

droplet’s picture

Status: Needs review » Closed (works as designed)

don't meant to change it