[Webkit-unassigned] [Bug 94671] Implement UndoManager's item() method

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=94671





--- Comment #4 from Kentaro Hara <haraken at chromium.org>  2012-08-22 16:58:20 PST ---
(From update of attachment 160028)
View in context: https://bugs.webkit.org/attachment.cgi?id=160028&action=review

It might make sense to use custom binding...

> Source/WebCore/bindings/v8/custom/V8UndoManagerCustom.cpp:70
> +        return v8::Null(args.GetIsolate());

I guess we normally throw some exception in such a case. (e.g. JavaScript RangeError or INDEX_SIZE_ERR of DOM exception etc). We might want to update the spec if needed.

> Source/WebCore/bindings/v8/custom/V8UndoManagerCustom.cpp:76
> +    for (size_t i = 0; i < entry.size(); ++i) {

Nit: i => index

>> Source/WebCore/bindings/v8/custom/V8UndoManagerCustom.cpp:82
>> +            object->ForceSet(v8::String::NewSymbol("label"), v8::String::NewSymbol("[Editing command]"));
> 
> We shouldn't be creating new object each time we return.
> e.g. if you do undoManager.item(0)[0].myProperty = 'foo';
> then I should be able to get 'foo' on
> undoManager.item(0)[0].myProperty
> even if item(0)[0] was an object added for an automatic transaction.

We don't want to use ForceSet() unless we have a strong reason to use it. Why isn't Set() enough?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.