Bug 109549

Summary: Turn avoidIntersectionWithNode into Editor member functions to encapsulate delete button controller
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: New BugsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, darin, dglazkov, enrica, gtk-ews, leviw, mifenton, ojan, peter+ews, tony, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 109534    
Bug Blocks: 109550    
Attachments:
Description Flags
Cleanup
none
Patch
none
Fixed build tony: review+

Description Ryosuke Niwa 2013-02-11 23:40:55 PST
Turn avoidIntersectionWithNode into Editor member functions to encapsulate delete button controller
Comment 1 Ryosuke Niwa 2013-02-11 23:53:02 PST
Created attachment 187785 [details]
Cleanup
Comment 2 EFL EWS Bot 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
Comment 3 Early Warning System Bot 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
Comment 4 Early Warning System Bot 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
Comment 5 kov's GTK+ EWS bot 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
Comment 6 WebKit Review Bot 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
Comment 7 WebKit Review Bot 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
Comment 8 Peter Beverloo (cr-android ews) 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
Comment 9 Ryosuke Niwa 2013-02-12 00:27:06 PST
Created attachment 187792 [details]
Patch
Comment 10 Early Warning System Bot 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
Comment 11 Early Warning System Bot 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
Comment 12 EFL EWS Bot 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
Comment 13 Ryosuke Niwa 2013-02-12 00:45:05 PST
Created attachment 187798 [details]
Fixed build
Comment 14 Tony Chang 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
Comment 15 Ryosuke Niwa 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.
Comment 16 Ryosuke Niwa 2013-02-12 18:40:53 PST
Committed r142705: <http://trac.webkit.org/changeset/142705>
Comment 17 Ryosuke Niwa 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.