Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#7925 closed enhancement (fixed)

[patch][ccla]dojo.hash: onhashchange handler

Reported by: Rob Retchless Owned by: Adam Peller
Priority: high Milestone: 1.4
Component: General Version: 1.2.0
Keywords: Cc: Rob Retchless
Blocked By: Blocking:

Description

Patch from Rob Retchless of IBM Rational (CCLA)

From Rob's description of what led them to their implementation:

We evaluated dojo.back for use in Jazz Web UIs, but found that it would be unsuitable for our needs because of one major loophole. Dojo.back keeps a history of states in a javascript object, meaning that the minute the browser is refreshed, all states in the travel log (history) are nuked. It is a very common use case, even within Ajax apps, for users to hit the refresh button. The expectation is that the back and forward buttons will continue to work

Dojo.hash does not rely on maintaining an object of browser states - it monitors for hash changes, and notifies connected callback functions when changes happen. It's up to consumers to figure out what the current hash is (simple call to window.location.hash), and to react to the change. This continues to work, even if users refresh in the middle of performing a complex series of back and forward operations.

dojo.hash also supports the onhashchange handler as a shim to the HTML5/IE8 spec.

Attachments (5)

dojohash.patch (9.0 KB) - added by Adam Peller 11 years ago.
dojo.hash impl and test from Rob Retchless (IBM, CCLA)
dojox-html-hash.zip (3.5 KB) - added by Adam Peller 11 years ago.
latest patch from Rob Retchless (IBM, CCLA) Tests will follow
dojo.hash-20090625.zip (3.4 KB) - added by Adam Peller 10 years ago.
update from Rob Retchless - a new cut of dojo.hash using string-only input, custom events and the HTML5 shim for using dojo.connect(window,"onhashchange", etc).
hash.patch (11.0 KB) - added by Adam Peller 10 years ago.
Rob's latest hash as a patch file with tests from John Ryding (also IBM, CCLA)
decode.patch (1.4 KB) - added by Adam Peller 10 years ago.
from Rob Retchless. Call decodeURIComponent again, tests pass.

Download all attachments as: .zip

Change History (45)

comment:1 Changed 11 years ago by Adam Peller

Cc: Rob Retchless added; retchles@… removed
Reporter: changed from Adam Peller to Rob Retchless

comment:2 Changed 11 years ago by Rob Retchless

I have posted a demo of this code at: http://retchless.com/hash/hash.html

Works in IE6+, FF2+, Safari 3+, Chrome.

Known issues:

  • Consumers must pass in true for the dontFix param in dojo.connect to stop it from trying to use attachEvent. (I'm hoping that someone can patch dojo.connect so that it detects whether the second parameter is a function, not an event, when the first parameter is a domNode)

Ex: dojo.connect(document.body, "onhashchange", context, callback, true)

  • Opera doesn't fire the onhashchange callback

comment:3 Changed 11 years ago by Rob Retchless

Turns out the Opera problem was a bug in my test case. Fixed.

Changed 11 years ago by Adam Peller

Attachment: dojohash.patch added

dojo.hash impl and test from Rob Retchless (IBM, CCLA)

comment:4 Changed 11 years ago by James Burke

Milestone: 1.3future

Looks like some interesting stuff in here. Initial feedback:

1) Looks like there are some small variances from the style guide, mostly the use of IFRAME_ID for a variable. I would just use iframeId. There is also some sort of weird character(s) at the beginning of tests/hash.html (at least in the trac-viewable version of the patch)?

2) Can we not have a isIE check to know if the onhashchange stub function is set up? Can we instead try to detect the existence of the function on the body object, and only create it if it does not exist? Hopefully that will help future browsers use the non-polling hash change stuff.

3) resources/hashiframe.html syncs the titles of the hidden iframe with the page title. Why is this desirable? Just wondering if we can cut out some JS work if we just give that HTML page a blank (or  ) title. 4) I would like to see this resolved in some way with dojo.back. Maybe dojo.back should depend on this module? We need to get dojo.back working for IE 8, maybe leveraging this module might help. It sounds like the dojo.back was found lacking, but dojo.hash does not fulfill the shortcoming -- dojo.hash seems to be lower level than dojo.back. Perhaps dojo.back needs to implement a hidden form to hold state, would that help with the original problem? Hmm, a form field solution would limit the state data to probably json objects. Anyway, can you share your thoughts on how we might explain/reconcile dojo.back with this module?

comment:5 Changed 11 years ago by James Burke

Milestone: future1.3

comment:6 Changed 11 years ago by Rob Retchless

#2, browser sniffing was the closest I was able to come on short notice, but I'd love to replace it with a better detection mechanism. If you know of a way to test whether the hashchange event will fire on document.body, I'm all ears!

#3, on IE, it's the title of the iframe that appears in the back history. So it's important to sync the titles.

#4, Sure, it's lower level, but that's all that's needed in many cases. It makes sense for dojo.back to build on top of this, but implementers should still be able to write their own state caching and hashchange reaction logic should they so desire.

comment:7 Changed 11 years ago by Rob Retchless

Some more thoughts on dojo.back + dojo.hash. In many cases, an implementer may just want one function to get fired no matter what history state they're at, and that function will do the URL parsing and figure out how to react. In this case, I wouldn't want a frameworky solution.. just dojo.hash. But there are two other features that dojo.back takes care: state caching and function mappings based on history state.

Essentially, I'm suggesting that dojo.back is a bit heavy handed, and should be split into building blocks, one of which being dojo.hash.

So there are three pieces:

1) a notification is fired when the hash changes (dojo.hash) 2) knowledge of what to execute when the hash changes (hash-to-action mapping) 3) a place to store application state (DOM nodes, data, etc) for each history entry

Across refreshes, it's fine (even expected) that state is nuked and repopulated, but the back/forward hash-to-action mappings (if defined) need to be maintained.

Is it safe to say that implementers will know what hash patterns are possible, and what they should execute, at page load time? If so, implementers could define mappings onload. Otherwise, the hidden form idea is an interesting one that deserves some looking into.

comment:8 Changed 11 years ago by James Burke

2) I was thinking of something like the following. We still have an IE check, but the code should upgrade nicely for newer browsers that support onhashchange (assuming the "in" check below works):

if(!("onhashchange" in dojo.body())){
    document.body.onhashchange = function(){ };  //create function stub to connect to
    if(dojo.isIE){
        //Use hidden iframe in older versions of IE 
        hash._IEUriMonitor = new IEUriMonitor(dojo.hitch(hash, hash._onHashChange));
    }else{
        setInterval(dojo.hitch(hash, hash._pollLocation), 200);
    }
}

2b) I also would probably try to change how IEUriMonitor() is used. Does not seem like we need to new one up, but rather it can be a local function we can just call, since IEUriMonitor is not visible outside this module. Similarly I would probably not pass in the _onHashChange dojo.hitch stuff, but just call that function directly inside IEUriMonitor, since IEUriMonitor's only use is bound to that method.

2c) It seems like hash.poke also needs to be defined based on if the native onhashchange is being used: in that case poke should be an empty function?

2d) I wonder if it useful having methods on dojo.hash at all. It seems like the user may only interact with dojo.hash.poke, everything else is in service of a setting up body.onhashchange. Can we even ditch poke()? In any case, I would just make the rest of the file local scoped variables and functions, to avoid having to worry about "this" in the code.

3) Right, forgot about that.

4) Yeah, sounds good, this module is a foundation layer.

comment:9 Changed 11 years ago by BillHiggins

@jburke:

Re: Your comments in 2b) about simplifying the code. All sounds good to me (I wrote quite a bit of this code).

Re: "can we ditch poke?" as Rob mentioned, we observed noticeable latency between link clicks and the attached function kicking in (typically updating the UI). We found the poke function necessary to eliminate this latency to make the UI's response to the user's click act and feel instantaneous. All of that being said, it would be possible to do something similar without a public poke function if you did the following:

1) Attach a generic click handler to body 2) In the click handler function body, test to see if the click was triggered from a link (problematic, because the target could be a span or some other element within a link) and if the link's href is only a hash change (i.e. protocol, host, port, uri are all the same 3) If all of the tests from 2) pass, manually fire the onhashchange event

I'm not recommending all this (it feels a tad presumptuous to me), but again, the latency is noticeable and I think we should either take care of it for users or provide an explicit way for users to avoid it.

Re: the iframe code content, this was the best I could come up with. If there is a simpler way to achieve the same result (updating titles), then I'm all for it. What you want to avoid though is not being able to update the title, otherwise you end up with a history like:

Jazz Jazz Jazz Jazz Jazz Jazz

... (what we used to have) which is a major usability problem.

comment:10 Changed 11 years ago by James Burke

@BillHiggins?:

OK, so maybe a dojo.hash.poke() is useful then. So that can be the only property on the dojo.hash object, the rest still seems like a good candidate for local variables/function work. Hmm, as far as the latency, thing, I had heard that human perception is good only down to 100ms. I wonder if you just change the polling to be 100 ms, does that help the issue? If not, then dojo.hash.poke() it is.

On the iframe contents, I wonder, can dojo.hash register its own listener for the onhashchange event, and use that callback to tell the iframe to update itself? That way we can piggyback on the one timer going on for the onhashchange stuff.

comment:11 Changed 11 years ago by Adam Peller

Milestone: 1.3future

the contributor doesn't have time to work this out for 1.3, and would like to work out a solution for dojo.back as well. will pursue in the future.

comment:12 Changed 11 years ago by Phil DeJarnett

I understand that this has been pushed off, but I really would love to see this implemented. The application I'm developing would benefit a lot more from being able to parse the hash directly instead of using dojo.back.

I quickly tested this under IE7 & IE8 (both standards and compatibility), FF3, Opera, and Safari4, and found that, after I updated the test to load the dojo.require inside the init function, it worked fine.

For now I applied the patch to my local copy to see how well it performs.

comment:13 Changed 11 years ago by Rob Retchless

We've resumed development on this feature and plan on contributing an updated patch in the coming weeks.

The plan is to contribute this to dojox.html with the following API:

dojox.html.addOnHashChange(callback), registers a callback to be called when the hash changes - when callback is called, an object containing the new hash as key/value pairs is passed as the first parameter.

dojox.html.parseHash(), returns an object with key/value pairs.

dojox.html.setHash(object), takes an object with key/value pairs and updates the URL.

Any code that would want to manipulate the hash non-destructively would do this:

function sortByTitle () {
   var hashObj = dojox.hash.parseHash();
   hashObj.sortTitle = 1;
   dojox.hash.setHash(hashObj);
}

comment:14 in reply to:  13 Changed 11 years ago by Phil DeJarnett

That's great news!

Replying to retchless:

dojox.html.parseHash(), returns an object with key/value pairs.

dojox.html.setHash(object), takes an object with key/value pairs and updates the URL.

While this is really cool, I would also like to see the ability to set the hash directly, instead of requiring the use of key/value pairs.

Possibly include dojox.html.getHash() to retrieve the unmanipulated hash. Then modify setHash to become dojox.html.setHash(/* String|Object */ value), where if dojo.isString(value) returns true, the hash is set directly.

This only adds one method (which is probably just not exposed), and a simple test to allow direct setting of the hash.

comment:15 Changed 11 years ago by Rob Retchless

Both great ideas! I will add these (getHash and polymorphic setHash).

comment:16 in reply to:  15 Changed 11 years ago by liucougar

Replying to retchless:

Both great ideas! I will add these (getHash and polymorphic setHash).

to go along with dojo.attr, you may want to consider: dojox.html.hash(/*?*/object)

if object is set, it is setHash(), otherwise it is parseHash(). In order to merge getHash() into this, in the object returned from parseHash(), a special key can be added (such as __raw__) which contains the unmanipulated hash

comment:17 Changed 11 years ago by Rob Retchless

@liucougar, I agree - unified getter/setter is incredibly elegant.

Progress report:

Dojo already has API for parsing query-type strings: dojo.queryToObject. So I'm thinking that we shouldn't make any assumptions on the structure or desired output - it should just return the string. If we want a parsed object, we can do this: dojo.queryToObject(dojox.html.hash());

Also, I've been thinking that passing the hash value as the first param to the callback is probably a bad idea because there's an event object in the IE8 case and we should probably pass that. The event object only exists if there is an onhashchange event in the browser, so most of the time this will be undefined. So as not to shoot ourselves in the foot in the future, we should leave it up to the consumer to call dojox.html.hash() in their callback.

Revised API:

dojox.html.hash(undefined | String | Array | Object)

  • gets or sets the hash string.
  • No arguments, returns a string, essentially: window.location.hash.substring(1);
  • String: sets hash directly, e.g. #string
  • Object: sets hash using query notation #param1=true&params2=true
  • Array: sets hash using slash notation #/param1/param2/param3

dojox.html.addOnHashChange(callback)

  • Registers a callback to be notified when the hash changes.
  • Passes the event object if it exists, or undefined.

comment:18 in reply to:  17 ; Changed 11 years ago by liucougar

Replying to retchless:

Dojo already has API for parsing query-type strings: dojo.queryToObject. So I'm thinking that we shouldn't make any assumptions on the structure or desired output - it should just return the string. If we want a parsed object, we can do this: dojo.queryToObject(dojox.html.hash());

that indeed sounds better

Also, I've been thinking that passing the hash value as the first param to the callback is probably a bad idea because there's an event object in the IE8 case and we should probably pass that. The event object only exists if there is an onhashchange event in the browser, so most of the time this will be undefined. So as not to shoot ourselves in the foot in the future, we should leave it up to the consumer to call dojox.html.hash() in their callback.

Revised API:

dojox.html.hash(undefined | String | Array | Object)

  • gets or sets the hash string.
  • No arguments, returns a string, essentially: window.location.hash.substring(1);

(IIRC, in some browser, window.location.hash does not starts with #)

  • String: sets hash directly, e.g. #string
  • Object: sets hash using query notation #param1=true&params2=true
  • Array: sets hash using slash notation #/param1/param2/param3

how about getting rid of support for Object/Array?, they can be easily achieved by: dojox.html.hash(dojo.objectToQuery(Object))
dojox.html.hash(Array.join('/')) (with this, user can easily changes the separator)

dojox.html.addOnHashChange(callback)

  • Registers a callback to be notified when the hash changes.
  • Passes the event object if it exists, or undefined.

when there is no event object, how about making one to mimic an event?

comment:19 in reply to:  18 Changed 11 years ago by Rob Retchless

Replying to liucougar:

how about getting rid of support for Object/Array?, they can be easily achieved by: dojox.html.hash(dojo.objectToQuery(Object))
dojox.html.hash(Array.join('/')) (with this, user can easily changes the separator)

I like the convenience of passing the raw objects. Plus it's only 2 extra lines of JS, and it's nice to not have to think about it.

when there is no event object, how about making one to mimic an event?

I'm not convinced the event object is really all that useful for onhashchange anyway, so mimicking it is probably overkill.

Changed 11 years ago by Adam Peller

Attachment: dojox-html-hash.zip added

latest patch from Rob Retchless (IBM, CCLA) Tests will follow

comment:20 Changed 11 years ago by Adam Peller

Milestone: future1.4

Changed 10 years ago by Adam Peller

Attachment: dojo.hash-20090625.zip added

update from Rob Retchless - a new cut of dojo.hash using string-only input, custom events and the HTML5 shim for using dojo.connect(window,"onhashchange", etc).

Changed 10 years ago by Adam Peller

Attachment: hash.patch added

Rob's latest hash as a patch file with tests from John Ryding (also IBM, CCLA)

comment:21 Changed 10 years ago by strife25

Are there any plans to backport this fix to version 1.3 when it is accepted?

comment:22 Changed 10 years ago by James Burke

strife25: it is unlikely it will be backported officially to 1.3 since it is a new feature, and depending on the implementation may require trunk code. But you might be able to grab it from 1.4 and use it in a 1.3 project if it stays self-contained.

comment:23 Changed 10 years ago by Phil DeJarnett

strife25: Just so you know, I'm using the original dojohash.patch applied to 1.3, and it works very well. I've had no issues in my testing. So, at least that is an option.

comment:24 Changed 10 years ago by liucougar

some suggestions on the latest hash.patch:

doc inconsistancy

dojo.hash = function(/* String | Object | Array */ hash)

hash can only be String now, so this signature needs to be changed

//	returns:
//		when used as a getter, returns the current hash string.
//		when used as a setter, undefined.

when used as a setter, the code actually returns the new hash just set, so the doc needs to be changed (or, maybe it's better to just return the original hash before this set?)

expose _pollFrequency

I think it would be good if _pollFrequency is exposed, or providing a way for user to overwrite it, so that it can be customized.

hashiframe.html

by setting the content of hashiframe.html to iframe.src, we don't need to load an extra file:

iframe.src="javascript:content_of_hashiframe.html"

by exposing content_of_hashiframe.html, we can allow user to customize it if desired

I am not sure whether that still works for IE, but I think it should

comment:25 Changed 10 years ago by rmaccracken

First I would like to say that I am so happy that you solved the history problem in this way because we have been using RSH and it seems like development for it has stopped.

So, I downloaded dojo.hash-20090625.zip and ran into a few problems with FF3.0.x and IE7. I was able to workaround both, so I figured I would post here to see what you think.

For FF3.0.x, I had a problem that the second time _fakeEvent was used it would throw an exception. It looked like the event changed after the first use - I'm not sure why that triggered an exception, but it did. So I fixed this simply by creating a new _fakeEvent each time it is needed rather than reusing a single one.

	function _pollLocation() {
		if (_getHash() === _recentHash) {	
			return;
		}
		_recentHash = _getHash();
		_fakeEvent = document.createEvent("HTMLEvents");
		_fakeEvent.initEvent("hashchange", true, false);
		document.body.dispatchEvent(_fakeEvent);
	}

For IE7, the problem I had was fairly simple to start. The file provides dojo.hash, but it tries to load the hashiframe.html resource from "dojox.html". I changed this to "dojo" - easy enough. But then I ran into problems with the following line:

document.body.onhashchange();

I am not sure why this generated an error, but commenting out the line was enough to make it work. I connect to the window onhashchange event, so I'm not sure why the document body onhashchange event is needed in the first place?

Anyways, just thought I would share my experiences.

comment:26 Changed 10 years ago by Rob Retchless

@liucougar:

Inlining the contents of the iframe doesn't work in IE7 unfortunately. At least not the way I was trying to do it.

I've updated the inline docs to better reflect the state of things.

Any ideas on how to expose poll frequency API? It's tricky because dojo.hash() is currently the only API.

@rmaccracken:

Thanks for pointing out the Firefox event reuse issue. I had this fixed in my local code, but hadn't updated the patch on this bug. New patch coming soon.

As for the IE7 issue, I can't reproduce. The reason it's calling document.body.onhashchange() as well as window.onhashchange() is that IE8 supports both. The spec alludes to the fact that the event should fire at the body and bubble to the window. So to fake that in IE, I'm stubbing an onhashchange function in both places, and then calling them sequentially.

comment:27 in reply to:  26 Changed 10 years ago by liucougar

Replying to retchless:

@liucougar: Any ideas on how to expose poll frequency API? It's tricky because dojo.hash() is currently the only API.

we can always stuff it in dojo.config (djConfig) :)

comment:28 in reply to:  26 Changed 10 years ago by liucougar

Replying to retchless:

@liucougar: Inlining the contents of the iframe doesn't work in IE7 unfortunately. At least not the way I was trying to do it.

it may be caused by the fact that you have js code in the iframe. I'd suggest a basically empty html for the iframe, and do the setInterval magic in dojo.hash module directly

hopefully that will work in IE7

comment:29 Changed 10 years ago by Adam Peller

(In [20110]) Land Rob Retchless' patch. Refs #7925 Will follow up with some sort of DOH tests, if possible.

comment:30 Changed 10 years ago by liucougar

the current way of detecting native onhashchange will not work in FF, while in FF 3.6, native onhashchange will be available.

may want to use http://thinkweb2.com/projects/prototype/detecting-event-support-without-browser-sniffing/ to detect native support

comment:31 in reply to:  30 Changed 10 years ago by Rob Retchless

Replying to liucougar:

the current way of detecting native onhashchange will not work in FF, while in FF 3.6, native onhashchange will be available.

I've tested this on the latest FF 3.6 nightly, and it picks up the native onhashchange support like a champ. In fact, all browsers that currently support onhashchange work as expected (FF3.6a2, IE 8, Chrome 4.0.206.1, and Webkit r48397).

Interesting link though - sounds like a good idea for a dojo feature.

comment:32 Changed 10 years ago by Adam Peller

(In [20122]) Update from Rob Retchless, IBM. Refs #7925

  • Fixed IE8 in quirks mode (seems that the onhashchange event is disabled in quirks even though testing for it says it exists #fail).
  • Massively improved IE6/IE7 performance (setTimeout 0 instead of _pollFrequency when waiting for iframe to catch up to main window).
  • Fixed tests

comment:33 Changed 10 years ago by Adam Peller

(In [20124]) Remove hash tests from module. Halts test suite. Refs #7925

comment:34 Changed 10 years ago by Adam Peller

Resolution: fixed
Status: newclosed

ok, I think this ticket is getting long and all the issues here have been addressed. If not, let's start opening up new tickets.

comment:35 Changed 10 years ago by bill

(In [20607]) Test was invalid since it ran doh.is() asynchronously without a try/catch block, causing an unhandled exceptionrather than calling d.errBack(). Was popping up error dialog on IE and hanging the regression.

Test is still failing on both IE and FF on "test with spaces".

Refs #7925.

comment:36 Changed 10 years ago by bill

Resolution: fixed
Status: closedreopened

Guys - you shouldn't be checking in tests that fail, and especially not tests that hang the test suite.

If you don't have a fix for the failing test then comment it out and open another ticket for it to be fixed in the future.

Changed 10 years ago by Adam Peller

Attachment: decode.patch added

from Rob Retchless. Call decodeURIComponent again, tests pass.

comment:37 Changed 10 years ago by Adam Peller

Priority: normalhigh

comment:38 Changed 10 years ago by Adam Peller

Resolution: fixed
Status: reopenedclosed

Fixed in [20642]

comment:40 Changed 10 years ago by Phil DeJarnett

WARNING: the link above is SPAM - potentially containing a VIRUS!

comment:40 Changed 10 years ago by Adam Peller

(In [21190]) Leave encoding/decoding up to user. Patch from retchless. Fixes #10595, Refs #7925 !strict

Note: See TracTickets for help on using tickets.