Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#938 closed defect (fixed)

Back called in continuous loop with changeURL used with bookmarking

Reported by: James Burke Owned by: James Burke
Priority: high Milestone:
Component: General Version: 0.2
Keywords: Cc: m.kaluzny@…
Blocked By: Blocking:

Description

In Firefox:

  • Go to tests/undo/test_browser.html#link1
  • Check the "Use Bookmark URL" checkbox.
  • Press the Link 2 button
  • Press Link 1 button
  • Press Browser Back Button
  • Press Link 1 button

The Back function is called continously. We should be clearing the forwardStack as new things are added to the history.

(Problem and solution first reported by Marek Kałużny)

I moved the this.forwardStack = []; to the start of addToHistory(), and that fixed the above test case, but I was able to get continuous back/forward calls using these steps:

  • Go to tests/undo/test_browser.html#link1
  • Check the "Use Bookmark URL" checkbox.
  • Press the Link 2 button
  • Press Link 1 button
  • Press Browser Back Button *2* times.
  • Press Link 1 button

I was hoping to fix this for 0.3.1, but it looks like there are some larger issues with bookmarked URLs (see also bug #907). The code needs to be reconsidered. My first thought is that Firefox should take the same path as MSIE for changeURL usage: just use the hidden iframe to track back/forward clicks, and don't do a timer that watches the URL. My quick, first pass at doing that didn't work, but there might be something to that approach. Punting to 0.4 (907 is also marked for 0.4).

Change History (5)

comment:1 Changed 13 years ago by James Burke

Owner: changed from anonymous to James Burke

comment:2 Changed 13 years ago by James Burke

Resolution: fixed
Status: newclosed

(In [5603]) Fixes #938, using changeUrl in Firefox with logic fragment identifier names had issues with back/forward.

comment:3 Changed 13 years ago by James Burke

The test cases above changed when I refactored the dojo.undo.browser tests. Here are the new test steps:

Test 1:

  • Go to tests/undo/test_browser_bookmark.html#xhr1
  • Press the XHR 2 button
  • Press XHR 1 button
  • Press Browser Back Button
  • Press XHR 1 button

Test 2:

  • Go to tests/undo/test_browser_bookmark.html#xhr1
  • Press the XHR 2 button
  • Press XHR 1 button
  • Press Browser Back Button *2* times.
  • Press XHR 1 button

comment:4 Changed 13 years ago by James Burke

I committed a fix, but I want to get a change in behavior reviewed. In Firefox, there is no way to detect changes if the same fragment identifier is used for multiple, adjacent requests to dojo.undo.browser.addToHistory(), since loading a page in an iframe doesn't work for this case and a timer is to check the hash.

I tried using something like location.watch() or checking window.history.length, but those things don't fire/change when the back/forward buttons are pressed.

So the fix: if the user calls dojo.undo.browser.addToHistory() with #state1 fragment identifier for the changeUrl parameter, and the previous addToHistory() call also used #state1, the second call's state object replaces the first object.

When not using dojo.undo.browser, and just clicking on buttons that change the fragment identifier, this behavior maps how Firefox and Safari work -- only one entry is placed in the history. MSIE in that case jumps completely out of the page, so I don't see a conflict with that behavior.

However, MSIE used to work OK if you called addToHistory() multiple times in a row with #state1. Now, with this bug fix change, it will work like Firefox/Safari?: only the last #state1 call's state object is kept. I felt it was more important to match behavior across browsers, even though it was a bit of "degradation" from MSIE's past dojo.undo.browser behavior (but that old code didn't even work for Firefox in this scenario).

If you feel this is not the way to go, please let me know.

comment:5 Changed 12 years ago by (none)

Milestone: 0.4

Milestone 0.4 deleted

Note: See TracTickets for help on using tickets.