Bug 109549 - Turn avoidIntersectionWithNode into Editor member functions to encapsulate delete button controller
Summary: Turn avoidIntersectionWithNode into Editor member functions to encapsulate de...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on: 109534
Blocks: 109550
  Show dependency tree / graph
 
Reported: 2013-02-11 23:40 PST by Ryosuke Niwa
Modified: 2013-02-12 20:11 PST (History)
13 users (show)

See Also:

Attachments
Cleanup (15.89 KB, patch)
2013-02-11 23:53 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch (15.94 KB, patch)
2013-02-12 00:27 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Fixed build (15.97 KB, patch)
2013-02-12 00:45 PST, Ryosuke Niwa 		tony: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.