Problem/Motivation

The toolbar doesn't direct users back to the homepage using the "Back to site" button.

To reproduce this bug:

  • Simply land on the front page of any fresh 8.x install.
  • Click away from the homepage, using the toolbar, to an administration page.
  • Observe the lack of "Back to site" button.

Obviously, the "Back to site" button is expected after leaving the homepage to visit an admin page.

Proposed resolution

Ensure that "Back to site" button is available, even if you entered an admin page from the front page.

Remaining tasks

  • Confirm the problem exists and is reproducible.
  • Review the provided patch.

User interface changes

The expected back-to-site button will show up in the normal place.

API changes

None.

Original report by @Sam_152

CommentFileSizeAuthor
#1 2195933-fix-back-to-site-button-for-homepage.patch664 bytesSam152
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Status: Active » Needs review
FileSize
2195933-fix-back-to-site-button-for-homepage.patch664 bytes

Patch attached.

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Issue summary: View changes
dcrocks’s picture

dcrocks CreditAttribution: dcrocks commented

I'm not 100% sure but I think #2194763: 'back to site' button doesn't work on site configured for 'dirty' url's is a duplicate of this. Can you give more info on what error you actually saw? Steps on how to reproduce would help.

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented

This is an entirely different issue. This isn't related to clean URLs or accessing index.php directly. Basically any time you leave the main website and venture into the admin panel, the state is saved so you can click "Back to site". Since the homepage is stored as an empty string, when the code included in the patch includes if (escapeAdminPath) it prevents the button from appearing because empty strings are falsey in JavaScript.

Since sessionStorage.getItem returns null for values that don't exist, we can safely update the condition to be more strict in it's checking.

dman’s picture

dman CreditAttribution: dman commented

Yeah, the problem definition here, succinctly described as :

Proposed resolution
Fix the problem.

Is hard to treat as an actionable "bug"

Needs:
much more clear functional, testable steps to reproduce
a description of the fail state and the expected success state.

Anything else cannot be evaluated without a huge number of unstated assumptions.

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Issue summary: View changes
Sam152’s picture

Sam152 CreditAttribution: Sam152 commented
Issue summary: View changes
Sam152’s picture

Sam152 CreditAttribution: Sam152 commented

Dman, thanks for the review of the issue summary. First time using the template, the "Proposed resolution" section seemed somewhat superfluous.

I have updated the issue with some more information to hopefully make this issue easier to follow.

dcrocks’s picture

dcrocks CreditAttribution: dcrocks commented

Tried to reproduce. When I run without clean url's, this doesn't happen. When I run with clean url's, this doesn't happen the 1st time I click on a toolbar admin tab, but does happen every time afterward. The process:

0. I'm running on OS X 10.6.
1. Clone a current dev copy of D8 into %username/Sites directory.
2. Modify .htaccess to turn on 'RewriteBase /~%username/drupal8'.
3. Run D8 install.
4. Sign in to drupal8. Toolbar is displayed at top of page.
5. Click on 'Content' tab and 'Back to site' button displayed.
6. Click on 'home'
7. Click on 'Content' tab again and 'Back to site button' is NOT displayed.
8. Repeat #6 and #7 n* times, with any any toolbar tab, and result always same, no 'Back to site' button.

Sign in and sign out and the behavior repeats.

dman’s picture

dman CreditAttribution: dman commented
Issue summary: View changes

Yeah I'm seeing this too. Can replicate.
This is indeed unexpected and a UI WTF.

I can confirm that this patch fixes it. as expected.
The code is a good fix.

dman’s picture

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

I was going to RTBC this when I reviewed, but was in a hurry so stepped back to consider if there was anything I hadn't thought of. Seeing as nothing has come to mind since then, I do vote this as good to go.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

Not a big deal but usually we compare things the other way around: escapeAdminPath !== null

Sam152’s picture

Sam152 CreditAttribution: Sam152 commented

I have a habit of using yoda conditions. Is there a concrete precedent on this? I can provide a reroll if necessary.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented

A precedent as in all the JS in core?

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Issue tags: +JavaScript
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

This was screwing me up last week when I was taking screenshots for a presentation. Happy to see this fixed! Thanks, Sam152!

Committed and pushed to 8.x, along with un-yodaing the condition. :) Thanks!

dman’s picture

dman CreditAttribution: dman commented

Bravo!
Good to see work by @Sam152 at the DrupalSouth code sprint getting in here.
FIRST TIME CORE CONTRIBUTOR! - You get a badge!
Thanks all!

Status: Fixed » Closed (fixed)

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