RESOLVED FIXED 109549
Turn avoidIntersectionWithNode into Editor member functions to encapsulate delete button controller 
https://bugs.webkit.org/show_bug.cgi?id=109549
Summary Turn avoidIntersectionWithNode into Editor member functions to encapsulate de...
Ryosuke Niwa
Reported 2013-02-11 23:40:55 PST
Turn avoidIntersectionWithNode into Editor member functions to encapsulate delete button controller
Attachments
Cleanup (15.89 KB, patch)
2013-02-11 23:53 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Patch (15.94 KB, patch)
2013-02-12 00:27 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fixed build (15.97 KB, patch)
2013-02-12 00:45 PST, Ryosuke Niwa 		tony: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2013-02-11 23:53:02 PST
Created attachment 187785 [details] Cleanup
EFL EWS Bot
Comment 2 2013-02-12 00:02:53 PST
Comment on attachment 187785 [details] Cleanup Attachment 187785 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/16521010
Early Warning System Bot
Comment 3 2013-02-12 00:03:06 PST
Comment on attachment 187785 [details] Cleanup Attachment 187785 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16454065
Early Warning System Bot
Comment 4 2013-02-12 00:03:38 PST
Comment on attachment 187785 [details] Cleanup Attachment 187785 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/16521009
kov's GTK+ EWS bot
Comment 5 2013-02-12 00:04:45 PST
Comment on attachment 187785 [details] Cleanup Attachment 187785 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/16520008
WebKit Review Bot
Comment 6 2013-02-12 00:13:07 PST
Comment on attachment 187785 [details] Cleanup Attachment 187785 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16439063
WebKit Review Bot
Comment 7 2013-02-12 00:14:37 PST
Comment on attachment 187785 [details] Cleanup Attachment 187785 [details] did not pass cr-linux-debug-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16515011
Peter Beverloo (cr-android ews)
Comment 8 2013-02-12 00:24:48 PST
Comment on attachment 187785 [details] Cleanup Attachment 187785 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/16520006
Ryosuke Niwa
Comment 9 2013-02-12 00:27:06 PST
Created attachment 187792 [details] Patch
Early Warning System Bot
Comment 10 2013-02-12 00:32:55 PST
Comment on attachment 187792 [details] Patch Attachment 187792 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16444087
Early Warning System Bot
Comment 11 2013-02-12 00:36:19 PST
Comment on attachment 187792 [details] Patch Attachment 187792 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/16519022
EFL EWS Bot
Comment 12 2013-02-12 00:38:48 PST
Comment on attachment 187792 [details] Patch Attachment 187792 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/16444090
Ryosuke Niwa
Comment 13 2013-02-12 00:45:05 PST
Created attachment 187798 [details] Fixed build
Tony Chang
Comment 14 2013-02-12 11:06:24 PST
Comment on attachment 187798 [details] Fixed build View in context: https://bugs.webkit.org/attachment.cgi?id=187798&action=review > Source/WebCore/editing/Editor.h:105 > + PassRefPtr<Range> avoidIntersectionWithDeleteButtonController(const Range* range) const { return range ? range->cloneRange(ASSERT_NO_EXCEPTION) : 0; } Maybe mention why we need to clone the range in the ChangeLog? > Source/WebCore/editing/markup.cpp:642 > + // Disable the delete button so it's elements are not serialized into the markup, Nit: it's -> its
Ryosuke Niwa
Comment 15 2013-02-12 12:56:03 PST
Comment on attachment 187798 [details] Fixed build View in context: https://bugs.webkit.org/attachment.cgi?id=187798&action=review Thanks for the review! >> Source/WebCore/editing/Editor.h:105 >> + PassRefPtr<Range> avoidIntersectionWithDeleteButtonController(const Range* range) const { return range ? range->cloneRange(ASSERT_NO_EXCEPTION) : 0; } > > Maybe mention why we need to clone the range in the ChangeLog? The only reason I need it is because we take const Range* :( But this code is never used because the only caller is createMarkup and the code is wrapped inside if-def ENABLE(DELETION_UI) so I'm gonna just delete this function for now.
Ryosuke Niwa
Comment 16 2013-02-12 18:40:53 PST
Committed r142705: <http://trac.webkit.org/changeset/142705>
Ryosuke Niwa
Comment 17 2013-02-12 20:11:17 PST
Apparently there was one more caller of Range* function :( Landed a crude fix in http://trac.webkit.org/changeset/142711.
Note You need to log in before you can comment on or make changes to this bug.