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:
- the root state should be recorded upon initialization: record the original
drupalSettings
and query the DOM for all regions, and store all their HTML - 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 thecontent
andheader
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:
/
/foo
/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.
Comment | File | Size | Author |
---|---|---|---|
#11 | 2713515-followup-11.patch | 462 bytes | Wim Leers |
#6 | 2713515-6.patch | 6.34 KB | Wim Leers |
Comments
Comment #2
Wim LeersThis would also fix backward navigation.
Imagine:
/
/foo
/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.
Comment #3
Wim LeersComment #4
Wim LeersHere is a fix for just point 1 in the IS.
Comment #5
Wim LeersAnd this implements points 2 in the IS.
Comment #6
Wim LeersDocs.
Comment #7
Wim LeersNot 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.
Comment #8
Wim LeersComment #10
Wim LeersComment #11
Wim LeersThere was one small oversight, that prevented it from actually working as intended.
Comment #13
Wim Leers