Keep align with the spec http://www.w3.org/TR/IndexedDB/#widl-IDBKeyRange-lowerBound-static-IDBKeyRange-any-lower-boolean-open
Created attachment 151423 [details] Patch
Comment on attachment 151423 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151423&action=review > Source/WebCore/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). can you please address this? Esp. give a justification why this change is required? > Source/WebCore/ChangeLog:10 > + No new tests (OOPS!). You should remove this line
Nice catch - thanks for noticing the issue and providing the patch. Looking through the IDB code and this patch, the patch and test updates look good to me.
> Looking through the IDB code and this patch, the patch and test updates look good to me. Apart from jochen@'s feedback on the ChangeLogs, of course. :)
Created attachment 151573 [details] Patch
(In reply to comment #5) > Created an attachment (id=151573) [details] > Patch Thanks for your review, patch updated as the comments.
This patch looks good to me
tony@ - could you r?
Comment on attachment 151573 [details] Patch (In reply to comment #2) > (From update of attachment 151423 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=151423&action=review > > > Source/WebCore/ChangeLog:10 > > + No new tests (OOPS!). > > You should remove this line Actually, you should replace it with a line explaining which tests cover this change or why it can't be tested. For this change, it should say something like: No new tests - updated storage/indexeddb/keyrange.html to match new behavior.
Created attachment 151839 [details] Patch
(In reply to comment #9) > (From update of attachment 151573 [details]) > (In reply to comment #2) > > (From update of attachment 151423 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=151423&action=review > > > > > Source/WebCore/ChangeLog:10 > > > + No new tests (OOPS!). > > > > You should remove this line > > Actually, you should replace it with a line explaining which tests cover this change or why it can't be tested. For this change, it should say something like: > No new tests - updated storage/indexeddb/keyrange.html to match new behavior. Got it, updated the patch added this line.
Comment on attachment 151839 [details] Patch Since Tony already reviewed this, you don't need to request another review If you fix below nit, I can set cq+ View in context: https://bugs.webkit.org/attachment.cgi?id=151839&action=review > Source/WebCore/ChangeLog:10 > + No new tests - updated storage/indexeddb/keyrange.html to match new behavior. nit. I would expect an empty line before this one
Created attachment 151875 [details] Patch
(In reply to comment #12) > (From update of attachment 151839 [details]) > Since Tony already reviewed this, you don't need to request another review > > If you fix below nit, I can set cq+ > > View in context: https://bugs.webkit.org/attachment.cgi?id=151839&action=review > > > Source/WebCore/ChangeLog:10 > > + No new tests - updated storage/indexeddb/keyrange.html to match new behavior. > > nit. I would expect an empty line before this one Done. Thank you.
Comment on attachment 151875 [details] Patch Clearing flags on attachment: 151875 Committed r122435: <http://trac.webkit.org/changeset/122435>
All reviewed patches have been landed. Closing bug.