Summary: | HTML5 Database stops executing transactions if the URL hash changes while a transaction is open and an XHR is in progress. | ||
---|---|---|---|
Product: | WebKit | Reporter: | Andrew Grieve <agrieve> |
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Major | CC: | abarth, benm, darin |
Priority: | P1 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Mac | ||
OS: | OS X 10.5 | ||
Attachments: |
Description
Andrew Grieve
2009-05-11 17:51:14 PDT
Created attachment 30211 [details]
Test harness for reproducing the bug.
Created attachment 30666 [details]
Patch to not stop databases when navigating the history to the same document with a fragment
This patch adds code to not stop the databases if we are navigating the history to a hash fragment on the same document and includes a layout test to verify. The test harness attached to this bug passes with this patch applied.
Comment on attachment 30666 [details]
Patch to not stop databases when navigating the history to the same document with a fragment
The layout test needs a little tweaking, will repost shortly.
Created attachment 30708 [details]
Patch to not stop the databases if the navigation is to a hash fragment on the same document.
New patch.
Comment on attachment 30708 [details]
Patch to not stop the databases if the navigation is to a hash fragment on the same document.
This change looks OK, but I'm disappointed about the way it uses state in the frame loader to make stopLoading not stop the databases. This seems a bit roundabout, making the already-fragile FrameLoader worse. I think that adding an argument to stopLoading would be better.
(In reply to comment #5) > (From update of attachment 30708 [details] [review]) > This change looks OK, but I'm disappointed about the way it uses state in the > frame loader to make stopLoading not stop the databases. This seems a bit > roundabout, making the already-fragile FrameLoader worse. I think that adding > an argument to stopLoading would be better. > OK, a new patch is one the way. Cheers, Ben Created attachment 30839 [details]
New patch with argument rather than member variable.
New patch with an argument to the function rather than a member variable tracking state.
Created attachment 30840 [details]
Replaces a couple of tabs with spaces in previous patch.
Comment on attachment 30840 [details] Replaces a couple of tabs with spaces in previous patch. > + void stopLoading(DatabasePolicy databasePolicy = DatabasePolicyStop); Can just say DatabasePolicy = DatabasePolicyStop -- no need for an argument name. r=me Will land. Sending LayoutTests/ChangeLog Adding LayoutTests/storage/hash-change-with-xhr-expected.txt Adding LayoutTests/storage/hash-change-with-xhr.html Sending WebCore/ChangeLog Sending WebCore/loader/DocumentLoader.cpp Sending WebCore/loader/DocumentLoader.h Sending WebCore/loader/FrameLoader.cpp Sending WebCore/loader/FrameLoader.h Sending WebCore/loader/FrameLoaderTypes.h Sending WebCore/page/Page.cpp Transmitting file data .......... Committed revision 44351. Patch applies but does not compile. Will revert. (In reply to comment #12) > Patch applies but does not compile. > Ah, sorry. It was fine on Windows but needed to change the WebCore exports file on Mac to reflect the new method signatures. Fixed with the new patch. Created attachment 30862 [details]
Updates WebCore.base.exp to fix build bustage.
Created attachment 30864 [details]
Fix build bustage.
Updates the WebCore exports.
Comment on attachment 30864 [details]
Fix build bustage.
Based on Darin's earlier review, this look good for landing
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::stopLoading):
+ * loader/DocumentLoader.h:
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::stopLoading):
+ (WebCore::FrameLoader::stopAllLoaders):
+ * loader/FrameLoader.h:
+ * loader/FrameLoaderTypes.h:
+ (WebCore::):
+ * page/Page.cpp:
+ (WebCore::Page::goToItem):
+ * WebCore.base.exp:
We much prefer detailed per-function comments in ChangeLogs.
Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/storage/hash-change-with-xhr-expected.txt A LayoutTests/storage/hash-change-with-xhr.html M WebCore/ChangeLog M WebCore/WebCore.base.exp M WebCore/loader/DocumentLoader.cpp M WebCore/loader/DocumentLoader.h M WebCore/loader/FrameLoader.cpp M WebCore/loader/FrameLoader.h M WebCore/loader/FrameLoaderTypes.h M WebCore/page/Page.cpp Committed r44468 |