Most of code in Cut and Copy functions in Editor.cpp is identical (or analogical). There is a FIXME line suggesting to refactor this part of the code to start sharing code.
Created attachment 226093 [details] Patch
Created attachment 226100 [details] There was small error in first version. Fixed.
Created attachment 226109 [details] Patch copy() and cut() functions have to be kept (calling newly refactored function) to avoid changing function names in all ports (which is not a good idea...)
Comment on attachment 226109 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=226109&action=review This patch looks OK, but I don’t want to say review+ because I don’t want to add a new enum and a new public function to Editor just to restructure the code internals. I’m going to say review-, but please don’t let that discourage you! > Source/WebCore/editing/Editor.cpp:1298 > + bool isDoneByDHTML = (action == CutAction) ? tryDHTMLCut() : tryDHTMLCopy(); > + if (isDoneByDHTML) > return; // DHTML did the whole operation > - if (!canCut()) { > + > + bool canPerformAction = (action == CutAction) ? canCut() : canCopy(); > + if (!canPerformAction) { > systemBeep(); > return; > } Not sure that putting this part of the function into shared code is worthwhile. I think cut and copy should just do these and then they could share a helper that does the rest. > Source/WebCore/editing/Editor.h:90 > +enum EditorActionSpecifier { CutAction, CopyAction }; This enum should be private to the Editor class rather than at the top level. The other enums are at the namespace level for convenient use outside the class, but this one does not need to be used outside the class. > Source/WebCore/editing/Editor.h:130 > + void performCutOrCopy(EditorActionSpecifier); This function should be private.
Created attachment 228033 [details] Patch All comments were very accurate and has been addressed.
> I’m going to say review-, but please don’t let that discourage you! It's not so easy to discourage me :) Especially when comments are good and accurate. Fixed and ready for next review.
Comment on attachment 228033 [details] Patch Clearing flags on attachment: 228033 Committed r166445: <http://trac.webkit.org/changeset/166445>
All reviewed patches have been landed. Closing bug.