The current root state handling is brittle and does not support fragment-based navigation: if the first link you click is a fragment link on the same page, then the root state will never be updated. Which means you can't actually navigate back to the very first Drupal page you visited… which means #2713439: Allow navigating back to the original page without doing a full reload was only a partial solution.

Once #2705363: Fragments URLs fail: local fragments cause unnecessary page load, remote fragments cause HTTP 500 response is fixed, we'll have the necessary infrastructure to reason about this better.

There are two things that should happen:

  1. the root state should be recorded upon initialization: record the original drupalSettings and query the DOM for all regions, and store all their HTML
  2. then when going backward, we should just compare the regions, and only revert those regions that were changed: when going from / to /foo, if /foo changed only the content and header regions, then when going back to /, we should only restore those two regions. We should NOT restore every region, because that's making more DOM changes than necessary.

Doing point 2 would also fix backward navigation.

Imagine the following navigation:

  1. /
  2. /foo
  3. /bar

If /foo changes regions A and B, and /bar changes regions B and C… then what we do today when going back from /bar to /foo is applying the HTML changes made for /foo… relative to /: we change regions A and B. Which means region C is left in the state for /bar!

A concrete example of this: create two published nodes, make sure they're both on the front page, and put one of the nodes also in the main navigation menu. Then go: 1) front page, 2) click on the node on the front page that is not in the menu, 3) click on the node in the menu. Now navigate back. RefreshLess will fail, because the primary_menu region should change, but is not changed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Issue summary: View changes

This would also fix backward navigation.

Imagine:

  1. /
  2. /foo
  3. /bar

If /foo changes regions A and B, and /bar changes regions B and C… then what we do today when going back from /bar to /foo is applying the HTML changes made for /foo… relative to /: we change regions A and B. Which means region C is left in the state for /bar!

That's very, very wrong.

Wim Leers’s picture

Issue summary: View changes
Wim Leers’s picture

Assigned: Unassigned » Wim Leers
Status: Active » Needs review
FileSize
3.27 KB

Here is a fix for just point 1 in the IS.

Wim Leers’s picture

FileSize
5.97 KB
2.97 KB

And this implements points 2 in the IS.

Wim Leers’s picture

FileSize
6.34 KB
1.32 KB

Docs.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
Related issues: +#2713567: Refactor all the controller logic into an actual object: improve understandability/maintainability

Not as clean code as I would like, but it'll be up to #2713567: Refactor all the controller logic into an actual object: improve understandability/maintainability to consolidate all that anyway. So, calling it done.

Wim Leers’s picture

Title: Fix backward navigation, improve root state handling » Fix backward navigation (in cases where different regions have been updated), improve root state handling

  • Wim Leers committed ea27218 on 8.x-1.x
    Issue #2713515 by Wim Leers: Fix backward navigation (in cases where...
Wim Leers’s picture

Assigned: Wim Leers » Unassigned
Status: Reviewed & tested by the community » Fixed
Wim Leers’s picture

Issue summary: View changes
Status: Fixed » Needs review
FileSize
462 bytes

There was one small oversight, that prevented it from actually working as intended.

  • Wim Leers committed 062a446 on 8.x-1.x
    Follow-up for issue #2713515 by Wim Leers: Fix backward navigation (in...
Wim Leers’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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