Problem/Motivation

All other Drupal sessionStorage and localStorage items are namespaced like "Drupal.[module].storageTokenName" but not "escapeAdminPath".

https://git.drupalcode.org/project/drupal/-/blob/9.2.x/core/modules/tool...

/**
 * @file
 * Replaces the home link in toolbar with a back to site link.
 */

(function ($, Drupal, drupalSettings) {
  const pathInfo = drupalSettings.path;
  const escapeAdminPath = sessionStorage.getItem('escapeAdminPath');
  const windowLocation = window.location;

Steps to reproduce

Proposed resolution

1.- Namespace it.

-   const escapeAdminPath = sessionStorage.getItem('escapeAdminPath');
+   const escapeAdminPath = sessionStorage.getItem('Drupal.toolbar.escapeAdminPath');

2.- Consider cleaning up the old un-namespaced item pollution.

Is there a policy for clearing old variable names from js app storage?

Remaining tasks

  1. write patch.
  2. review.

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Release notes snippet

CommentFileSizeAuthor
#9 interdiff-7_9.txt487 bytesgauravvvv
#9 3205499-9.patch877 bytesgauravvvv
#7 3205499-7.patch609 bytesgauravvvv

Issue fork drupal-3205499

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jwilson3 created an issue. See original summary.

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.

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

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

gauravvvv’s picture

gauravvvv commented
Status: Active » Needs review
StatusFileSize
new 3205499-7.patch609 bytes

I have attached patch for same, please review

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Seems to have some failures.

gauravvvv’s picture

gauravvvv commented
Status: Needs work » Needs review
StatusFileSize
new 3205499-9.patch877 bytes
new interdiff-7_9.txt487 bytes

Tried fixing failures, attached interdiff for same

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems fine.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs review

Looks like there are couple contrib modules using this session storage entry. Should we deprecate the old key before removing it?

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Seems minor but probably the safest route

yash.rode’s picture

yash.rode commented

How can we apply javascript deprecation on top of this session storage key?

yash.rode opened merge request !8412

yash.rode’s picture

yash.rode commented
Status: Needs work » Needs review
utkarsh_33’s picture

utkarsh_33 commented
Status: Needs review » Needs work

Just a small change.

yash.rode’s picture

yash.rode commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs change record

Deprecation key seems straight forward. But should the message include a link to a CR or mention what should be used instead?

utkarsh_33’s picture

utkarsh_33 commented
Status: Needs work » Needs review

Added the CR.Thanks!

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record +Needs Review Queue Initiative

Short and to the point!

Kernel failure appeared to be random, re-ran the tests all are green.

Believe this one is good to go.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Reviewed & tested by the community » Needs work

CR was a bit too short, added a bit of context :) Since this is sessionStorage there is no need to get rid of the old variable, it will be deleted when the tab closes.

I know it's now in the toolbar module but it's something that is used in gin, will be used by the navigation module eventually, and the toolbar module will be deprecated/removed at some point. So not sure it's necessary to namespace it in the toolbar. We'd need to do another deprecation later and we don't need to go through that I think.

Can we do Drupal.escapeAdminPath instead?

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.