|Summary:||multiple problems prevent bookmarking/back button technique for AJAX/DHTML applications from working|
|Product:||WebKit||Reporter:||Brad Neuberg <bkn3>|
|Severity:||Major||CC:||adele, alice.barraclough, andersca, bkn3, bugs-webkit, christopher, darin, ddkilzer, eric, Kevin, mjs, sjoerdmulder, sjt, trey|
|OS:||OS X 10.4|
|Bug Depends on:||7058|
|Bug Blocks:||9610, 6628|
Description Brad Neuberg 2005-12-31 02:46:39 PST
Comment 1 Dave Hyatt 2006-01-31 14:48:59 PST
If you have any simple reduced test cases (if you can host them) would be very helpful. The issues with window.location.href are pretty well known, but reading your linked blog entry, I didn't quite follow what your issue was with iframes/forms.
Comment 2 Dave Hyatt 2006-01-31 14:58:22 PST
Inifnite loop = p1. This is probably related to 6958.
Comment 3 Brad Neuberg 2006-01-31 15:08:32 PST
Comment 4 Brad Neuberg 2006-01-31 15:10:04 PST
By the way, it doesn't matter how you handle iframes and history if you support the process above. IE also doesn't work correctly with updating the anchor location (it doesn't always go into history), so IE needed to use a deeper hack using hidden iframes to register history events (it's nasty).
Comment 5 Dave Hyatt 2006-01-31 15:18:50 PST
I'm pretty sure we intended to put # URLS into the session history. Probably just a bug (or design flaw) that we lose intermediate ones. Our session history code is 3 years old and Firefox's is 7, so it's unsurprising that we have more bugs. :)
Comment 6 Dave Hyatt 2006-01-31 15:20:28 PST
BTW whenever you have problems with Safari/WebKit, you can come to #webkit on Freenode to ask for help. No need to beat against something for 2 weeks when you can ask for help from the actual developers.
Comment 7 David Kilzer (:ddkilzer) 2006-01-31 20:35:31 PST
Created attachment 6173 [details] Test case Test case for setting window.location.href using relative links.
Comment 8 David Kilzer (:ddkilzer) 2006-01-31 21:59:17 PST
Created attachment 6174 [details] Patch v1 (probably not final fix) Poking around kjs_window.cpp, I found that the change in this patch would fix the attached test case (Attachment 6173 [details]). The change from calling scheduleLocationChange() to changeLocation() causes the Redirection Timer not to spin endlessly after the test case is done. The change from "!userGesture" to "false" in the third argument causes the history to be written for each change to the URL fragment/reference (#locationX). I have no idea if this is the correct fix, though, since it takes a while for the "Back" history button to update after the test case is run, and the test case seems to reload the entire page every time. It seems like there is a missing optimization where loading a URL fragment (#locationX) on the same page shouldn't force the entire page to be reloaded.
Comment 9 Darin Adler 2006-01-31 22:22:42 PST
Comment on attachment 6174 [details] Patch v1 (probably not final fix) We definitely can't do this. It sill introduce all sorts of problems to try to actually change the location right away. Sadly, I don't know exactly which problem it will cause -- it would be great to have the test cases, but I don't think we do.
Comment 10 Trey Matteson 2006-01-31 23:13:32 PST
I would expect that passing "false" instead of "!userGesture", as proposed in the patch, would break a large number of sites that implement redirects using pure JS. In fact, the attached test case of changing the location in onload() is a perfect example. We solved a lot of conflicting bugs when we realized the existing "userGesture" flag in khtml was the way to distinguish whether window.location=foo should add to the history. You might try a test case that acts in response to a user event to see if it changes the behavior of TOT. If I'm right, I realize this situation might not be good enough to meet your original goals.
Comment 11 Brad Neuberg 2006-02-01 11:33:01 PST
By the way, setting an anchor location should _not_ reload the page, reset the DOM, etc. Other browsers don't do this.
Comment 12 Darin Adler 2006-02-02 10:20:09 PST
(In reply to comment #11) > By the way, setting an anchor location should _not_ reload the page, reset the > DOM, etc. Other browsers don't do this. Good point. Maybe we can break that part of this out into a separate bug report. This one seems to be about a set of related issues, and so it's a little hard to understand and fix.
Comment 13 Darin Adler 2006-02-02 10:21:03 PST
In general, the most helpful thing here would be to break this up into discrete pieces that are each easy to understand.
Comment 15 David Kilzer (:ddkilzer) 2006-02-04 12:25:51 PST
Comment 16 David Kilzer (:ddkilzer) 2006-02-04 19:38:13 PST
(In reply to comment #15) > 1. The RedirectionTimer should NOT fire when requesting a '#name' URL on the > same page. Hmm...it would seem that Bug 3552 covers this issue!
Comment 17 David Kilzer (:ddkilzer) 2006-02-07 21:21:30 PST
This bug was fixed with the commit for Bug 7058 (confirmed locally by applying the patch in reverse and rebuilding r12652), so marking it a duplicate of that bug. Bug 3552 was also fixed at the same time. Reporter, please verify that this was fixed with nightly r12598 or newer. Should a test case be prepared and checked in for WebKit to make sure this doesn't regress? *** This bug has been marked as a duplicate of 7058 ***
Comment 18 Sjoerd Mulder 2006-02-08 01:26:59 PST
This bug is definitly NOT a duplicate of bug 7058! This bug is about Safari supporting back / forward button in AJAX applications, I agree it's come one step closer by fixing bug 7058 but it still has some problems! Attaching a better testcase! (where you actually have to use the history by pressing Back)
Comment 19 Sjoerd Mulder 2006-02-08 01:30:50 PST
Created attachment 6339 [details] Testcase using Back button
Comment 20 Brad Neuberg 2006-02-08 01:48:12 PST
Here is a test page using the Really Simple History library which works on IE and Firefox: http://www.onjava.com/onjava/2005/10/26/examples/examples/oreillymail/oreillymail.html You can use this to make sure that your fixes work. This is a simple, fake AJAX/DHTML email client put together to show AJAX history, bookmarking, and back/button support. What you should do is click around on the left hand sidebar; make sure that the page location changes at the top, showing a new anchor location. Then, press the back and forward buttons, and make sure that the history changes in the anchor bar. Press the reload button and make sure the page stays the same, then press back and forward again to make sure the history is still there. Manually change the anchor location to a new place (such as #drafts), then press enter; the page should correctly change; continue using the history to make sure this manual change doesn't break things, by clicking on the sidebar more. Then, go to http://google.com in this window, let Google load, and then press the back button to jump back to the test page; you should be on the last history location you left it at. Continue pressing the back button to jump through your history state, ensuring that leaving the page and then returning has not cleared out your AJAX history. This all works in IE and Firefox. It should work in Safari. Google has adopted this library for some of their own projects, so getting it working in Safari would be nice.
Comment 21 David Kilzer (:ddkilzer) 2006-02-08 05:06:12 PST
Adding Bug 7058 to depends-on list since it fixed part of the issue originally described in the description of this bug. I apologize for missing the "location.hash does not get updated" issue when I wrote the initial test case (Attachment 6173 [details]), but "(sometimes you don't get the hash value)" in the original description didn't inform me that "location.hash" wasn't getting updated. I thought that meant the hash value wasn't available in location.href. In the future if you want Software X or Web Site Y to work with WebKit and there are multiple individual issues that are able to be identified, please set up a "Master" or "Tracking" bug (like Bug 6628 for Backbase or Bug 6627 for TinyMCE) and then reference existing bugs or create additional bugs (with test cases!) that block the master/tracking bug. Thanks!
Comment 22 Kevin Newman 2006-04-11 11:10:57 PDT
Comment 23 Darin Adler 2006-04-11 12:02:37 PDT
The most helpful thing anyone could do about this bug would be to write some small specific bug reports. This giant one about a whole category of problems is very difficult to work on. Individual specific ones, each with a small, specific set of steps would be a lot easier to resolve.
Comment 24 Anders Carlsson 2006-05-27 05:03:29 PDT
I'm working on this. I have a patch which fixes most problems, it just needs some more testing
Comment 25 Darin Adler 2006-05-30 10:57:44 PDT
Anders says he's working on this, so assigning to him to reflect that fact.
Comment 26 Anders Carlsson 2006-06-01 07:51:59 PDT
Created attachment 8642 [details] Patch This patch fixes the issues described above
Comment 27 Darin Adler 2006-06-01 13:00:36 PDT
Comment on attachment 8642 [details] Patch Since == already works on KURL, it seems to me that we only need a function that does the "== ignoring hash" operation. Also, for symmetric operations like equality, I prefer functions to members. So I'd like to see the isEqualTo function renamed and remove the boolean parameter and be a "free" function rather than a member function. In historyURL: we should not use braces around single-line if statements. Otherwise looks good, and those are nitpicks, so r=me.
Comment 28 David Kilzer (:ddkilzer) 2006-06-01 17:38:34 PDT
*** Bug 8522 has been marked as a duplicate of this bug. ***
Comment 29 David Kilzer (:ddkilzer) 2006-06-02 09:53:05 PDT
*** Bug 7516 has been marked as a duplicate of this bug. ***
Comment 30 David Kilzer (:ddkilzer) 2006-06-02 09:57:52 PDT
*** Bug 9184 has been marked as a duplicate of this bug. ***
Comment 31 David Kilzer (:ddkilzer) 2006-06-02 20:43:18 PDT
*** Bug 9281 has been marked as a duplicate of this bug. ***
Comment 32 David Kilzer (:ddkilzer) 2006-06-02 20:51:00 PDT
*** Bug 9266 has been marked as a duplicate of this bug. ***