[Webkit-unassigned] [Bug 93912] Implement UndoManager's V8 bindings

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Ryosuke Niwa <rniwa at webkit.org>  2012-08-16 21:45:30 PST ---
(From update of attachment 158982)
View in context: https://bugs.webkit.org/attachment.cgi?id=158982&action=review

> Source/WebCore/editing/UndoManager.cpp:62
> +        if (entry[0]->isDOMTransaction()) {

Isn't it possible for someone to execute execCommand as is?
which means that we still have to check isDOMTransaction for each item in the entry, no?

> Source/WebCore/editing/UndoManager.cpp:174
> +void UndoManager::undo()
> +{
> +    undo(ASSERT_NO_EXCEPTION);
> +}
> +
> +void UndoManager::redo()
> +{
> +    redo(ASSERT_NO_EXCEPTION);
> +}

You can just use ASSERT_NO_EXCEPTION as the default argument value.

> LayoutTests/editing/undomanager/domtransaction-survives-gc.html:21
> +    "execute": function() { result += ("execute "); },

Why do we need parenthesis around "execute "?

> LayoutTests/editing/undomanager/domtransaction-survives-gc.html:31
> +if (result == "execute undo ")
> +    log.innerHTML += "PASS: after the GC ran, undoManager.undo() still worked.<br>";
> +else
> +    log.innerHTML += "FAIL: after the GC ran, undoManager.undo() did not work.<br>";

You can include js-test-pre and use testPassed / testFailed.
Better yet, shouldBe("gc(); document.undoManager.undo(); result", "execute undo ");

> LayoutTests/editing/undomanager/undomanager-isolated-world.html:13
> +if (window.testRunner) {
> +    testRunner.dumpAsText();

We typically output an error message like "this test requires testRunner" when the test requires testRunner and cannot be ran manually.

> LayoutTests/editing/undomanager/undomanager-isolated-world.html:30
> +    var log = document.getElementById('log');
> +    if (log.innerHTML == "")
> +        log.innerHTML = "PASS: undoManager's callback was not invoked in isolated world."

Ditto about using js-test-pre.js.

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