Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client side HTML5 History API support #72

Merged
merged 25 commits into from Jun 28, 2012

Conversation

vesse
Copy link
Contributor

@vesse vesse commented Jan 18, 2012

Enable using HTML5 History API and the clean URLs it brings instead of the hash fragment URIs on browsers that support it. In practice a new configure option html5history was added and the required changes implemented to browser.js.

These changes have been manually tested with:

  • Chrome 16.0.912.75 (Windows)
  • Chromium 15.0.874.106 (Linux)
  • Firefox 9.0.1 (Linux & Windows)
  • Safari 5 (Windows)
    • Navigation from the UI works with Safari as well, but browser back button skips pages. This does happen with e.g. Spine.js as well, and also on http://html5demos.com/history so I would assume it's just a browser "feature".
  • Internet Explorer 9.0.8.8112.16421 (64-bit)
  • rekonq 0.8.0 (location bar URI does not change but navigation works)

No tests were added due to not being able to test with just a HTML page and QUnit. Implementing tests would require adding a server implementation that serves the test page and related JS components.

Documentation is updated as well. Unfortunately the diff contains a lot of irrelevant changes as my editor stripped trailing whitespaces.
#20

Vesa Poikajärvi added 6 commits January 17, 2012 13:49
Added support for configure option html5history and related browser side
implementation. With this option routing can use pretty URIs instead of
the hash URIs. It's worth to notice that switching the method most
likely requires changes also to backend since the browser will, upon
initial page load, ask for the URI it is loading instead of the service
index (e.g. yourhost.com/users/1).

There is a bug in Chrome which causes it to trigger onpopstate event
when performing initial page load. Since the event handler must be
called upon init manually to ensure correct content, the bug would cause
two handler calls in Chrome. Now the handler is set in a setTimeout in
order to avoid (most) double calls in Chrome.

See Chrome bug for reference:
http://code.google.com/p/chromium/issues/detail?id=63040
Also a lot of fuzz due to my editor stripping out extra whitespace.
At least IE9 returns undefined instead of null and thus history support
would be incorrectly set to true if using strict comparison.
@pksunkara
Copy link
Contributor

Thanks, will be reviewed soon.

@indexzero
Copy link
Member

Cool. I'm no expert on pushState implementations, but @jashkenas was asking about this last week at NYC.js. Awesome how open source works like that :)

@markmarkoh
Copy link

Any update on whether or not this is making it into director? Currently using vesse's branch in the meantime.

@coderarity
Copy link
Contributor

Do want.

@mrienstra
Copy link
Contributor

+1

Vesa Poikajärvi added 3 commits February 28, 2012 17:44
In some cases, like when you get an URL from an IE user, page can be entered with a hash URL. This commit changes the functionality so that the hash is checked regardless of operation mode, and if in HTML5 history mode the current history state is replaced and handler is called with a nice, clean URI.
@pksunkara
Copy link
Contributor

@hij1nx Decision please.

@heapwolf
Copy link

I'm on the fence about taking this commit without any tests. although i really want these features. Is there any way you can add a simple harness with a director backend?

@vesse
Copy link
Contributor Author

vesse commented Mar 28, 2012

@hij1nx If your question was meant for me, sure. My original intention was of course to add tests as well but then I got busy, and also didn't know what would be the preferred way from this project's point of view - i.e. would a simple, manually launched node server and manually opening the browser test page suffice. If it does I'll try to find some time to implement the tests.

Vesa Poikajärvi added 9 commits May 10, 2012 13:30
By default when in html5history mode the route handler is executed in Router.init() because of need for routing cannot be determined (like it easily can when using hashes). Usually this is the wanted behavior, but since that is not always the case this option makes it possible to disable execution.
@vesse
Copy link
Contributor Author

vesse commented May 11, 2012

OK, finally got time to implement the dummy backend and tests. There is now file director/test/browser/backend/backend.js that needs to be started in that folder. After that going to http://localhost:8080/ executes the tests that are basically the same as the hash routing tests.

I needed to modify helpers/api.js (didn't want to add options to every call plus a timeout is needed due to that Chrome bug mentioned in the actual browser.js) a bit but the old tests should work.

When implementing the tests I also added yet another configuration option run_handler_in_init (sorry for the lame name, didn't come up with anything better) which works only if combined with html5mode. If that option is set to false then the route handler is not executed in Router.init(), but by default it is because of not being able to determine if init should route or not.

@indexzero
Copy link
Member

@vesse Thanks for taking the time to write tests. I know we want this feature and with these tests I think @hij1nx will be more receptive :-D

Vesa Poikajarvi added 2 commits May 11, 2012 13:02
…ng enabled since the outcome depends on the uri where the browser is when test starts
Added hash routing fallback to the route handlers to enable testing with
IE. Not all tests pass, but that is the case with test-harness.html as
well so supposedly this is not because of html5 mode.
@Marak
Copy link
Contributor

Marak commented May 20, 2012

+1

Vesa Poikajärvi added 4 commits June 19, 2012 14:54
Conflicts:
	README.md
	lib/director/browser.js
	test/browser/routes-harness.html
Async testing and setTimeouts needed for Chrome bug made the test highly
unreliable. It failed randomly depending on if the route could be
changed to '/a' before executing the test.
@framp
Copy link

framp commented Jun 27, 2012

Is there any reasons for this not being merged to director?

If there is something which needs to be done I'll be happy to help as this feature is pretty nice and Vesse's implementation is working flawlessly!

@pksunkara
Copy link
Contributor

@framp Will review and finish this by tomorrow.

heapwolf pushed a commit that referenced this pull request Jun 28, 2012
[lib] Client side HTML5 History API support
@heapwolf heapwolf merged commit 2452258 into flatiron:master Jun 28, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants