[webkit-reviews] review granted: [Bug 55640] Cursor.continue with a key param should test less than, not equal to : [Attachment 84504] Patch

[bugzilla-daemon at webkit.org]

Steve Block <steveblock at google.com> has granted Jeremy Orlow
<jorlow at chromium.org>'s request for review:
Bug 55640: Cursor.continue with a key param should test less than, not equal to
https://bugs.webkit.org/show_bug.cgi?id=55640

Attachment 84504: Patch
https://bugs.webkit.org/attachment.cgi?id=84504&action=review

------- Additional Comments from Steve Block <steveblock at google.com>
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-conti
nue
>
http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-steps-for-iterat
ing-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.