[webkit-reviews] review granted: [Bug 67173] Add a test for lastChangeWasUserEdit in HTMLInputElement and HTMLTextAreaElement : [Attachment 105557] adds a basic test

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 67173: Add a test for lastChangeWasUserEdit in HTMLInputElement and
HTMLTextAreaElement
https://bugs.webkit.org/show_bug.cgi?id=67173

Attachment 105557: adds a basic test
https://bugs.webkit.org/attachment.cgi?id=105557&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=105557&action=review


> Source/WebCore/testing/Internals.cpp:275
> +    if (HTMLInputElement* input = textField->toInputElement())

Wow, never saw this function before. Having toInputElement is crazy. Did we
really add that?! Why have this for one specific element type?

> LayoutTests/fast/forms/textfield-lastchange-was-useredit.html:13
> +if (!window.layoutTestController || !window.internals)

This is the only line of code that needs to say "window." in its expression.

> LayoutTests/fast/forms/textfield-lastchange-was-useredit.html:22
> +    shouldBeFalse('window.internals.wasLastChangeUserEdit(textField)');

Just internals would be fine.

> LayoutTests/fast/forms/textfield-lastchange-was-useredit.html:26
> +    textField.value = 'hello';
> +    debug('\nAfter setting value');
> +    shouldBeFalse('window.internals.wasLastChangeUserEdit(textField)');

Another way to write this is:

    shouldBeFalse("textField.value = 'hello';
internals.wasLastChangeUserEdit(textField)");

Using lines like that makes the test output less wordy than all those debug
statements.

> LayoutTests/fast/forms/textfield-lastchange-was-useredit.html:59
> +	   textField.textContent = "hello\nworld";
> +	   debug('\nAfter modifying textContent');
> +	   shouldBeFalse('window.internals.wasLastChangeUserEdit(textField)');

Maybe should test modifying innerText too?