Updated: Comment #0

Problem/Motivation

When using the toolbar 'Back to site' button, the URL returning to is not keeping the original language prefix or the querystring context.

Steps to reproduce:

  1. Starting from a multilingual site, with an adequate number of nodes published to front page to have a pager represented.
  2. Navigate to a language prefixed home; from the list of nodes select a page different than first: you end up with a URL like
    http://[...]/it/node?page=2
  3. Select any admin page from the toolbar.
  4. Click on the 'Back to site' button.
  5. The URL of the returning to page is
    http://[...]/node

    , so losing both language context and the actual page of the URL I left to get to the admin pages.

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#5 core_2176721_5.patch1 KBmaartendeblock
#3 core_2176721_3.patch712 bytesmaartendeblock
#14 core-js-ui-autocomplete-2186647-13.patch1021 bytesdroplet
#11 core-js-escapeadmin-lang-2176721-11.patch1.03 KBnod_
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Priority: Normal » Major
Issue tags: +JavaScript

Borderline critical.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Issue summary: View changes
Related issues: +#2146035: drupalSettings.path.pathPrefix does not contain the language identifier

Probably, #2146035: drupalSettings.path.pathPrefix does not contain the language identifier is related.

maartendeblock’s picture

maartendeblock CreditAttribution: maartendeblock commented
Assigned: Unassigned » maartendeblock
Status: Active » Needs review
FileSize
core_2176721_3.patch712 bytes

drupalSettings.pathPrefix is indeed empty. But we don't really need it for this functionality. Al we need to know is the basePath and remove it from the current request. This makes it safe for all urls and aliases, with or without pathPrefix or anything else.

I made a patch to fix this.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs review » Needs work

Since window.location is used in 3 places could you put that in a variable somewhere and use it instead of refering to window.location ?
Also the pathInfo variable is drupalSetting.path, no need to explicitly call that one.

Thanks!

maartendeblock’s picture

maartendeblock CreditAttribution: maartendeblock commented
Status: Needs work » Needs review
FileSize
core_2176721_5.patch1 KB

Like this?

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

yep, all good, thanks!

Status: Needs review » Needs work

The last submitted patch, 5: core_2176721_5.patch, failed testing.

The last submitted patch, 3: core_2176721_3.patch, failed testing.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs work » Needs review

Cancelled patch test, it's only changing JS files.

socketwench’s picture

socketwench CreditAttribution: socketwench commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs manual testing

Patch applies cleanly, and corrects the issue. Confirmed by manual testing.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Reviewed & tested by the community » Needs review
FileSize
core-js-escapeadmin-lang-2176721-11.patch1.03 KB

This is a reroll because the indentation standard changed for JavaScript files. No need for commit credit because of this reroll.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

droplet’s picture

droplet CreditAttribution: droplet commented
Assigned: maartendeblock » Unassigned
Status: Fixed » Needs review
FileSize
core-js-ui-autocomplete-2186647-13.patch1021 bytes

EDIT: Sorry, FILENAME is wrong, but the patch is correct

What if the entry point is a admin page, e.g.: "drupal8x/admin/content"
(Please make sure you don't have escapeAdminPath on your broswer sessionStorage while you do the testing.)

** Also, why doing it from frontend JS ?? Doesn't it should handle by backend toolbar module ?? (that will reduce frontend JS and less reflow.)

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs review » Needs work

Last patch seem wrong though :p

This should have been fixed by #2194763: 'back to site' button doesn't work on site configured for 'dirty' url's.

droplet’s picture

droplet CreditAttribution: droplet commented

maybe I should open a new issue for it.

I assumed if you enter "drupal8x/admin/" directly in address bar, it still show "Back to site" link ??

If Yes, my patch still correct.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

It's just that the patch has nothing to do with the issue title.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers commented
Status: Needs work » Closed (fixed)

Please file a new issue rather than reopening only tangentially related existing ones. Thanks!