Bug 28417

Summary: openDatabase() with empty version sets db version up incorrectly
Product: WebKit Reporter: Ian 'Hixie' Hickson <ian>
Component: New BugsAssignee: Ben Murdoch <benm>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, beidson, benm, commit-queue, ddkilzer, michaeln, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Proposed patch none

Ian 'Hixie' Hickson
Reported 2009-08-17 17:55:28 PDT
The following code should throw an exception on the second openDatabase() call, but doesn't: var n = 'test ' + new Date(); var db1 = openDatabase(n, '', '', 0); var db2 = openDatabase(n, 'test', '', 0); STEPS TO REPRODUCE 1. Open http://software.hixie.ch/utilities/js/js-eval-window/ 2. Paste the code above 3. Click the button EXPECTED RESULTS It should say "Error: INVALID_STATE_ERR: DOM Exception 11". ACTUAL RESULTS It says "undefined". It works fine if the first openDatabase() call's second argument is not the empty string. This may be due to some confusion about the empty string argument to openDatabase() -- per spec, the empty string sets the version to the empty string if the database doesn't exist, but otherwise just accepts whatever version is there.
Attachments
Proposed patch (5.42 KB, patch)
2009-10-13 10:10 PDT, Ben Murdoch 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ben Murdoch
Comment 1 2009-10-13 10:06:48 PDT
I have a fix and layout test for this bug, will send the patch shortly. I've run the layout tests on Windows and Mac (debug) and with the patch all the old tests continue to pass.
Ben Murdoch
Comment 2 2009-10-13 10:10:44 PDT
Created attachment 41112 [details] Proposed patch
Ben Murdoch
Comment 3 2009-10-27 13:02:42 PDT
ping...
Michael Nordman
Comment 4 2009-10-30 09:13:32 PDT
fwiw, lgtm
Ben Murdoch
Comment 5 2009-10-30 09:27:24 PDT
(In reply to comment #4) > fwiw, lgtm Thanks Michael. cc'ing ddkilzer who reviewed the last DB patch I sent. David, would you mind looking at this? Thanks!
Adam Barth
Comment 6 2009-10-30 11:12:08 PDT
The code looks fine, but I don't know which is the proper behavior.
David Kilzer (:ddkilzer)
Comment 7 2009-10-30 12:44:00 PDT
(In reply to comment #6) > The code looks fine, but I don't know which is the proper behavior. Considering that the bug was filed by the author of HTML5, I think the correct behavior is as stated in Comment #0.
David Kilzer (:ddkilzer)
Comment 8 2009-10-30 13:03:24 PDT
Comment on attachment 41112 [details] Proposed patch > Index: LayoutTests/storage/open-database-set-empty-version.html > =================================================================== > --- LayoutTests/storage/open-database-set-empty-version.html (revision 0) > +++ LayoutTests/storage/open-database-set-empty-version.html (revision 0) > @@ -0,0 +1,26 @@ > +<html> > +<head> > +<script> > +function runTest() { > + if (window.layoutTestController) { > + layoutTestController.dumpAsText(); > + layoutTestController.clearAllDatabases(); > + } > + > + try { > + var db = openDatabase('28417Test', '', 'Test for bug 28417: openDatabase() with empty version sets db version up incorrectly', 0); > + // The next openDatabase call should fail because the database version was set to '' by the call above, and now we are expecting a different version. > + var db2 = openDatabase('28417Test', 'test', 'Test for bug 28417: openDatabase() with empty version sets db version up incorrectly', 0); > + } catch (e) { > + document.getElementById('result').innerHTML = 'SUCCESS, an exception was thrown. ' + e; > + } The "var db = ..." statement could be moved outside the try{} block in case it generates an exception (although I suppose it's somewhat unlikely that it would generate the same exception and make the test pass when it shouldn't). Might be nice to close the databases when you're done with them, too, but not necessary. r=me
WebKit Commit Bot
Comment 9 2009-10-30 13:20:41 PDT
Comment on attachment 41112 [details] Proposed patch Clearing flags on attachment: 41112 Committed r50350: <http://trac.webkit.org/changeset/50350>
WebKit Commit Bot
Comment 10 2009-10-30 13:20:47 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.