Following up on Darin's comments in bug #20780.: We should rename the fields RenderTextControl::m_edited and RenderTextControl::m_userEdited to be more descriptive.
Created attachment 43020 [details] Patch No functionality was changed so no tests were included in this patch. On Darin's suggestion <https://bugs.webkit.org/show_bug.cgi?id=20780#c24>, I chose to rename RenderTextControl::m_edited to wasChangedSinceLastChangeEvent, and RenderTextControl::m_userEdited to m_lastChangeWasUserEdit. I felt these names were consistent (in wording - both use the word "change" to describe the modification to the text control) and best described what these fields are for. It would have been nice if we could have more concise names for these fields, but I could not think of any. We should probably look into re-factoring the functionality of these methods/cleanup RenderTextControl as outlined in <https://bugs.webkit.org/show_bug.cgi?id=20780#c14>. Then we may have a cleaner API.
Comment on attachment 43020 [details] Patch > - // We set m_userEdited to false since this change was not explicty made by the user (say, via typing on the keyboard), see <rdar://problem/5359921>. > - m_userEdited = false; > + // We set m_lastChangeWasUserEdit to false since this change was not explicty made by the user (say, via typing on the keyboard), see <rdar://problem/5359921>. > + m_lastChangeWasUserEdit = false; It would be nice to fix the spelling of explicitly. I don't think the "see rdar" comment is really all that useful, even for people at Apple. I think the comment is no longer needed; the new name of the data member speaks for itself. Or if we really want one, the comment could say, "Set m_lastChangeWasUserEdit to false since this function is only used for programmatic changes, not for user editing." As you mentioned, we should make other changes here. We don't want people to directly set these with set functions.
I'll just remove the comment when I land this, since the name of the field implies that we are using it to differentiate programmatic changes from user editing. (In reply to comment #2) > (From update of attachment 43020 [details]) > > - // We set m_userEdited to false since this change was not explicty made by the user (say, via typing on the keyboard), see <rdar://problem/5359921>. > > - m_userEdited = false; > > + // We set m_lastChangeWasUserEdit to false since this change was not explicty made by the user (say, via typing on the keyboard), see <rdar://problem/5359921>. > > + m_lastChangeWasUserEdit = false; > > It would be nice to fix the spelling of explicitly. > > I don't think the "see rdar" comment is really all that useful, even for people > at Apple. > > I think the comment is no longer needed; the new name of the data member speaks > for itself. Or if we really want one, the comment could say, "Set > m_lastChangeWasUserEdit to false since this function is only used for > programmatic changes, not for user editing." > > As you mentioned, we should make other changes here. We don't want people to > directly set these with set functions.
Comment on attachment 43020 [details] Patch Dan to edit before landing.
Committed r50999: <http://trac.webkit.org/changeset/50999>
Committed r51000: <http://trac.webkit.org/changeset/51000>
Comment on attachment 43020 [details] Patch Clearing review flag since Daniel will be doing a new patch that covers the rest of the call sites.
Commit in r50999 but failed to build on the Win build bots. So, rolled it out in r51000. Will correct the Windows build issues.
Re-opening bug since we need to fix this issue.
Created attachment 43256 [details] Patch Makes my Windows build smile :-)
Comment on attachment 43256 [details] Patch Changing the name of the function in DOMPrivate.idl will break compilation of Safari for Windows, although it doesn't break the compilation of WebKit itself. I suggest you take the same approach with Windows that you did with Mac OS X. Leave the interface of WebKit for clients outside WebKit with the old name. Change only the names internal to WebKit. So the DOMPrivate.idl name would need to stay the same, as would DOMHTMLClasses.h, and the function names in DOMHTMLClasses.cpp.
Created attachment 43259 [details] Patch Updated patch based on Darin's comment.
Comment on attachment 43259 [details] Patch Clearing flags on attachment: 43259 Committed r51148: <http://trac.webkit.org/changeset/51148>
All reviewed patches have been landed. Closing bug.