setInnerTextValue shouldn't live in RenderTextControl as it's nothing to do with the renderer. Since its sole purpose is to update the innerTextElement's children, it's more appropriate for it to live in HTMLTextFormControlElement.
This is also a left over from WML :(
Created attachment 105527 [details] Patch
Created attachment 105528 [details] made setLastChangeWasNotUserEdit protected
Comment on attachment 105528 [details] made setLastChangeWasNotUserEdit protected View in context: https://bugs.webkit.org/attachment.cgi?id=105528&action=review > Source/WebCore/ChangeLog:17 > + This patch also fixes the bug that lastChangeWasUserEdit() incorrectly returns false > + when input or textarea don't have renderer (e.g. invisible). Should we have a test for this bug?
(In reply to comment #4) > (From update of attachment 105528 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=105528&action=review > > > Source/WebCore/ChangeLog:17 > > + This patch also fixes the bug that lastChangeWasUserEdit() incorrectly returns false > > + when input or textarea don't have renderer (e.g. invisible). > > Should we have a test for this bug? We can't because it's only exposed as WebKit API (no Web-facing API).
(In reply to comment #5) > (In reply to comment #4) > > > + This patch also fixes the bug that lastChangeWasUserEdit() incorrectly returns false > > > + when input or textarea don't have renderer (e.g. invisible). > > > > Should we have a test for this bug? > > We can't because it's only exposed as WebKit API (no Web-facing API). We can expose this to DumpRenderTree in the tools itself or via window.internals. That’s what we’ve done in the past.
For an API, the right thing to do is to expose it in LayoutTestController, I think. That way, we can test WebKit layer, too. Internals are just for stuff that's not exposed by WebKit.
> (In reply to comment #5) > We can expose this to DumpRenderTree in the tools itself or via window.internals. That’s what we’ve done in the past. (In reply to comment #7) > For an API, the right thing to do is to expose it in LayoutTestController, I think. That way, we can test WebKit layer, too. Internals are just for stuff that's not exposed by WebKit. Okay, let's do this in a separate patch.(In reply to comment #6)
(In reply to comment #7) > For an API, the right thing to do is to expose it in LayoutTestController, I think. That way, we can test WebKit layer, too. Internals are just for stuff that's not exposed by WebKit. That’s not quite how I understand it. Internals is a way to test things that are not exposed by WebKit, and also has the advantage that a single test hook that works on all platforms. Exposing things in LayoutTestController allows us to test all the WebKit code too, but is much more work to support cross-platform. Further, some things might be exposed in WebKit on some platforms and not others. So Internals lets you test WebCore, not WebKit. And LayoutTestController can be used to test more of WebKit. I think in this case, there are arguments both ways about what the best way to write the test is.
I'm personally inclined towards exposing it via WebCore because the value of lastChangeWasUserEdit is unlikely to be affected by WebKit code.
(In reply to comment #10) > I'm personally inclined towards exposing it via WebCore because the value of lastChangeWasUserEdit is unlikely to be affected by WebKit code. s/WebCore/window.internals/
Comment on attachment 105528 [details] made setLastChangeWasNotUserEdit protected View in context: https://bugs.webkit.org/attachment.cgi?id=105528&action=review Please do your best to make the Windows build work by editing the .def file. EWS might have enough information for you to do that. > Source/WebCore/html/HTMLTextAreaElement.cpp:336 > + setLastChangeWasNotUserEdit(); Really could land this bug fix separately. Why not do that? > Source/WebCore/html/HTMLTextFormControlElement.h:103 > + void setLastChangeWasNotUserEdit() { m_lastChangeWasUserEdit = false; } > private: Should have a blank line here.
Comment on attachment 105528 [details] made setLastChangeWasNotUserEdit protected View in context: https://bugs.webkit.org/attachment.cgi?id=105528&action=review >> Source/WebCore/html/HTMLTextAreaElement.cpp:336 >> + setLastChangeWasNotUserEdit(); > > Really could land this bug fix separately. Why not do that? It might break lastChangeWasUserEdit if I do that. I'll add some method on internals in a separate patch so that we can test this feature.
I'll land this patch once r94038 is settled and I can confirm that this patch doesn't break lastChangeWasUserEdit.
Committed r94047: <http://trac.webkit.org/changeset/94047>
Committed r94049: <http://trac.webkit.org/changeset/94049>