Problem/Motivation

The navigation top bar is not rendering consistently (i.e. sometimes the top bar is rendered, sometimes not). It seems that this is due to insufficient cache contexts.

Steps to reproduce

  1. Enable navigation top bar. Module was hidden in #3486198: Hide the navigation top bar in 11.1.x and 10.4.x, so should be done via drush
  2. Navigate to a page on the frontend without top bar (e.g., /node)
  3. Navigate to a page that's supposed to render top bar (e.g., node/1)
  4. Notice that top bar is not rendering

The top bar should render as expected if you remove step 3. and navigate directly to a page that's supposed to render top bar.

Proposed resolution

Modify PageActions plugin to ensure that route cache context is included, even if the page don't have local tasks. That will avoid caching issues when the 1st page visited does not belong to an entity.

Remaining tasks

Implement solution
Implement tests

CommentFileSizeAuthor
#19 navigation-top-bar-not-rendered-consistently-3491044_19.patch4.39 KBf0ns
#14 Screenshot 2024-12-13 at 11.55.01 AM.png255.25 KBsmustgrave
#14 Screenshot 2024-12-13 at 11.53.51 AM.png492.01 KBsmustgrave
#8 Topbar.png196.26 KBsagarmohite0031

Issue fork drupal-3491044

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

lauriii created an issue. See original summary.

lauriii opened merge request !10420

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Version: 11.1.x-dev » 11.x-dev
Issue summary: View changes
Status: Active » Needs work
Issue tags: +Needs tests

plopesc made their first commit to this issue’s fork.

plopesc opened merge request !10422

plopesc changed the visibility of the branch 3491044-navigation-top-bar to hidden.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
Issue tags: -Needs tests

New MR following a different approach where context responsibility relies on the plugins instead the Top Bar itself.

sagarmohite0031’s picture

sagarmohite0031 commented
StatusFileSize
new Topbar.png196.26 KB

Hello,

Tested and Verified on Drupal 11
Mr applied successfully
Attaching screenshot-

vensires’s picture

vensires
Location Greece
commented

@sagarmohite0031, so is this RTBC?

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Can the steps to reproduce be updated. Navigation_top bar isn't there anymore.

vensires’s picture

vensires
Location Greece
commented

@smustgrave the navigation_top_bar module is still there but marked with hidden: true.
Is it ok to set it back to "Needs review"?

smustgrave’s picture

smustgrave commented
Status: Needs work » Needs review

Sure

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new Screenshot 2024-12-13 at 11.53.51 AM.png492.01 KB
new Screenshot 2024-12-13 at 11.55.01 AM.png255.25 KB

@plopsec pinged me about this one in slack. And as mentioned navigation_top_bar is now hidden. So re-tested

I did verify, with caching enabled, that on node/1 the top bar is not appearing

Before

before

After applying the patch

after

Toolbar is now appearing as expected.

1) Drupal\Tests\navigation\Functional\NavigationTopBarTest::testTopBarVisibility
Behat\Mink\Exception\ElementNotFoundException: Element matching xpath "//div[contains(@class, 'top-bar__content')]/div[contains(@class, 'top-bar__actions')]/button/span" not found.
/builds/issue/drupal-3491044/vendor/behat/mink/src/WebAssert.php:465
/builds/issue/drupal-3491044/core/modules/navigation/tests/src/Functional/NavigationTopBarTest.php:104
FAILURES!
Tests: 1, Assertions: 19, Failures: 1.
Exiting with EXIT_CODE=1

Test-only feature shows this too

  • nod_ committed 79304e95 on 11.x 
    Issue #3491044 by plopesc, lauriii, smustgrave: Navigation top bar is...
nod_’s picture

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

Committed 79304e9 and pushed to 11.x. Thanks!

nod_ closed merge request !10422

Status: Fixed » Closed (fixed)

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

f0ns’s picture

f0ns commented
StatusFileSize
new navigation-top-bar-not-rendered-consistently-3491044_19.patch4.39 KB

I had the same issue and the MR fixed it for me in the form of a patch (in attachment for others that also need it).

flyke’s picture

flyke commented

Thank you for the patch @f0ns