Updated: Comment 0

Problem/Motivation

@See image and title

toolbar broken

If you have your language as part of the URL the main request generates cache IDs per language, though the subtree access fails.

Proposed resolution

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
4.64 KB

The problem is that the langcode does not get passed along to the subtree request. This seems to be one way to solve it. Maybe though we could also somehow encode this directly into the hash.

YesCT’s picture

Issue tags: +D8MI, +JavaScript

adding d8mi since it has to do with languages.
adding JavaScript tag since it touches a javascript file, and @nod_ asked all issues that touch js files get that tag. (#2219493: Add search for issues with patches that touch certain file types (javascript, css, ...) or files automates/helps with that)

Wim Leers’s picture

Assigned: Unassigned » jessebeach

Assigning to Jesse Beach for feedback.

Gábor Hojtsy’s picture

Issue tags: +language-ui

Looks good on a code review.

+++ b/core/modules/toolbar/toolbar.module
@@ -561,12 +566,13 @@ function toolbar_user_role_update(RoleInterface $role) {
+ * @param string $langcode
+ *   The langcode of the current request.
  * @return string
  *   A unique cache ID for the user.

The only thing I noticed on code review is a missing empty line between @param and @return.

jessebeach’s picture

ifrik and I added a new test and extend the testSubtreesJsonRequest to include langcode. We've included a fail version with just the extended testing and a new patch with the testing.

The last submitted patch, 5: toolbar-langcode-2218313-5.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: toolbar-langcode-403-2218313-tests-fail.patch, failed testing.

Sam152’s picture

olli’s picture

jessebeach’s picture

Status: Needs work » Reviewed & tested by the community

Thank you for the cross link olli!

I consider the code in this issue sufficient to address the original bug report. dawehner's patch in #1 solved the issue and #5 just adds additional test coverage.

I'll leave a note in #2146035: drupalSettings.path.pathPrefix does not contain the language identifier to look specifically at the Toolbar code if that issue goes further.

  • Commit ddc0b0b on 8.x by alexpott:
    Issue #2218313 by jessebeach, dawehner: Toolbar subtree is broken if you...
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed ddc0b0b and pushed to 8.x. Thanks!

diff --git a/core/modules/toolbar/toolbar.module b/core/modules/toolbar/toolbar.module
index bfa71bf..0f58cb2 100644
--- a/core/modules/toolbar/toolbar.module
+++ b/core/modules/toolbar/toolbar.module
@@ -573,6 +573,7 @@ function toolbar_user_role_update(RoleInterface $role) {
  *   A user ID.
  * @param string $langcode
  *   The langcode of the current request.
+ *
  * @return string
  *   A unique cache ID for the user.
  */

Fixed during commit

Status: Fixed » Closed (fixed)

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