A Refactoring to tackle Bug 26526.
Created attachment 54364 [details] v0; a small change preparing for Bug 26526
Comment on attachment 54364 [details] v0; a small change preparing for Bug 26526 WebCore/html/HTMLFormControlElement.h:160 + virtual void defaultEventHandler(Event*); This should be as private as possible. I don't see any good reason to make it public. Either protected or private should work. Similarly, I think HTMLInputElement and HTMLTextAreaElement could now have private defaultEventHandler functions since there is no longer anyone calling directly. WebCore/rendering/TextControlInnerElements.cpp:119 + if (shadowAncestor && shadowAncestor->renderer()) { + ASSERT(shadowAncestor->renderer()->isTextControl()); We should not fetch the renderer() any more or assert that it is a text control. review- because I think it's worth refining this a tiny bit more, since its entire purpose is the refactoring.
Created attachment 54651 [details] v1; took feedback
Created attachment 54652 [details] v1; took feedback
Comment on attachment 54651 [details] v1; took feedback Oops, this is wrong.
Comment on attachment 54652 [details] v1; took feedback This is also wrong. I'm sorry to bother you ...
Created attachment 54663 [details] v1; took feedback
Hi Darin, thank you for reviewing and sorry for noisy attachments. (In reply to comment #2) > (From update of attachment 54364 [details]) > WebCore/html/HTMLFormControlElement.h:160 > + virtual void defaultEventHandler(Event*); > This should be as private as possible. I don't see any good reason to make it > public. Either protected or private should work. Similarly, I think > HTMLInputElement and HTMLTextAreaElement could now have private > defaultEventHandler functions since there is no longer anyone calling directly. Fixed moving defaultEventHandler() declarations from public to protected or private (based on inheritance relation) > WebCore/rendering/TextControlInnerElements.cpp:119 > + if (shadowAncestor && shadowAncestor->renderer()) { > + ASSERT(shadowAncestor->renderer()->isTextControl()); > We should not fetch the renderer() any more or assert that it is a text > control. Fixed.
Comment on attachment 54663 [details] v1; took feedback > + ASSERT(!shadowAncestor->renderer() || shadowAncestor->renderer()->isTextControl()); I'm not sure this assertion is helpful or needed. This code doesn't have any reason to care about the type of the renderer. r=me regardless
Attachment 54663 [details] was posted by a committer and has review+, assigning to MORITA Hajime for commit.
Committed r58864: <http://trac.webkit.org/changeset/58864>