Bug 55640 – Cursor.continue with a key param should test less than, not equal to

• Component: New Bugs
• Status: RESOLVED FIXED

[Jeremy Orlow]

Cursor.continue with a key param should test less than, not equal to

[Jeremy Orlow]

Created attachment 84504 [details]
Patch

[Jeremy Orlow]

Steve, can you please review?

[Steve Block]

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?

[Jeremy Orlow]

(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+?

[Steve Block]

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.

[Jeremy Orlow]

Committed r80308: <http://trac.webkit.org/changeset/80308>