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

CommentFileSizeAuthor
#5 interdiff.txt2.45 KBjessebeach
#5 toolbar-langcode-403-2218313-tests-fail.patch2.94 KBjessebeach
#5 toolbar-langcode-2218313-5.patch7.09 KBjessebeach
#1 toolbar-2218313-1.patch4.64 KBdawehner
00002182.png878.58 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new toolbar-2218313-1.patch4.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

yesct commented
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

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » jessebeach

Assigning to Jesse Beach for feedback.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
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

jessebeach commented
StatusFileSize
new toolbar-langcode-2218313-5.patch7.09 KB
new toolbar-langcode-403-2218313-tests-fail.patch2.94 KB
new interdiff.txt2.45 KB

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

sam152 commented

5: toolbar-langcode-2218313-5.patch queued for re-testing.

olli’s picture

olli commented

Looks like #2146035: drupalSettings.path.pathPrefix does not contain the language identifier could fix this issue.

jessebeach’s picture

jessebeach commented
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

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
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.