Bug 55640 - Cursor.continue with a key param should test less than, not equal to
Summary: Cursor.continue with a key param should test less than, not equal to
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-02 18:30 PST by Jeremy Orlow
Modified: 2011-03-03 18:19 PST (History)
4 users (show)

See Also:


Attachments
Patch (14.76 KB, patch)
2011-03-02 18:32 PST, Jeremy Orlow
steveblock: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2011-03-02 18:30:57 PST
Cursor.continue with a key param should test less than, not equal to
Comment 1 Jeremy Orlow 2011-03-02 18:32:01 PST
Created attachment 84504 [details]
Patch
Comment 2 Jeremy Orlow 2011-03-02 18:32:45 PST
Steve, can you please review?
Comment 3 Steve Block 2011-03-03 12:50:13 PST
Comment on attachment 84504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84504&action=review

> LayoutTests/storage/indexeddb/cursor-continue.html:38
> +    "the BIGEST string"

BIGGEST

> LayoutTests/storage/indexeddb/cursor-continue.html:90
> +    // OOPS: Don't land. Hack.

???

> Source/WebCore/ChangeLog:8
> +        If you supply a param to cursor.continue, we sould guarentee that

should guarantee

> Source/WebCore/ChangeLog:10
> +        Right now, we only test equality.

Do you have a link to this in the spec?

> Source/WebCore/storage/IDBCursorBackendImpl.cpp:137
> +        // If a key was supplied, we must loop until we find a key greater than or equal to it (or hit the end).

Given the 'else' below, this comment seems to describe only half the story, or am I missing something?
Comment 4 Jeremy Orlow 2011-03-03 13:27:37 PST
(In reply to comment #3)
> (From update of attachment 84504 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84504&action=review
> 
> > LayoutTests/storage/indexeddb/cursor-continue.html:38
> > +    "the BIGEST string"
> 
> BIGGEST

Will fix.
 
> > LayoutTests/storage/indexeddb/cursor-continue.html:90
> > +    // OOPS: Don't land. Hack.
> 
> ???

Don't worry, will remove before I land.  (The version I was developing against didn't have that patch yet.

> > Source/WebCore/ChangeLog:8
> > +        If you supply a param to cursor.continue, we sould guarentee that
> 
> should guarantee

Will do

> > Source/WebCore/ChangeLog:10
> > +        Right now, we only test equality.
> 
> Do you have a link to this in the spec?

http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBCursor-continue
http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-steps-for-iterating-a-cursor

> > Source/WebCore/storage/IDBCursorBackendImpl.cpp:137
> > +        // If a key was supplied, we must loop until we find a key greater than or equal to it (or hit the end).
> 
> Given the 'else' below, this comment seems to describe only half the story, or am I missing something?

I'd hope that a reader would look at the rest of it as well.  I guess I could make it a bit more clear.


Do you have any substantive concerns?  If not, I can haz r+?
Comment 5 Steve Block 2011-03-03 14:37:14 PST
Comment on attachment 84504 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84504&action=review

>>> LayoutTests/storage/indexeddb/cursor-continue.html:38
>>> +    "the BIGEST string"
>> 
>> BIGGEST
> 
> Will fix.

BIGGEST

>>> LayoutTests/storage/indexeddb/cursor-continue.html:90
>>> +    // OOPS: Don't land. Hack.
>> 
>> ???
> 
> Don't worry, will remove before I land.  (The version I was developing against didn't have that patch yet.

???

>>> Source/WebCore/ChangeLog:10
>>> +        Right now, we only test equality.
>> 
>> Do you have a link to this in the spec?
> 
> http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#widl-IDBCursor-continue
> http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-steps-for-iterating-a-cursor

OK, would be good to add this to the bug URL field or the ChangeLog

>>> Source/WebCore/storage/IDBCursorBackendImpl.cpp:137
>>> +        // If a key was supplied, we must loop until we find a key greater than or equal to it (or hit the end).
>> 
>> Given the 'else' below, this comment seems to describe only half the story, or am I missing something?
> 
> I'd hope that a reader would look at the rest of it as well.  I guess I could make it a bit more clear.
> 
> 
> Do you have any substantive concerns?  If not, I can haz r+?

Sure, but the bug and ChangeLog only mention the forward case, so I wanted to check that I understood correctly.
Comment 6 Jeremy Orlow 2011-03-03 18:19:00 PST
Committed r80308: <http://trac.webkit.org/changeset/80308>