Bug 109550 - DeleteButtonController::enable and disable should be called via a RAII object
Summary: DeleteButtonController::enable and disable should be called via a RAII object
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on: 109549
Blocks:
  Show dependency tree / graph
 
Reported: 2013-02-12 00:03 PST by Ryosuke Niwa
Modified: 2013-02-15 13:53 PST (History)
11 users (show)

See Also:

Attachments
Cleanup (8.56 KB, patch)
2013-02-13 09:25 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Cleanup (8.08 KB, patch)
2013-02-13 09:25 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Fixed build (8.11 KB, patch)
2013-02-13 10:17 PST, Ryosuke Niwa 		enrica: 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-12 00:03:18 PST 
DeleteButtonController::enable/disable should be wrapped in a RAII object.

Introducing such a RAII object will also allow us to further reduce the number of if-defs for ENABLE_DELETION_UI.
Comment 1 Ryosuke Niwa 2013-02-13 09:25:15 PST 
Created attachment 188097 [details]
Cleanup
Comment 2 Ryosuke Niwa 2013-02-13 09:25:48 PST 
Created attachment 188098 [details]
Cleanup
Comment 3 WebKit Review Bot 2013-02-13 09:31:05 PST 
Comment on attachment 188098 [details]
Cleanup

Attachment 188098 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16538176
Comment 4 Early Warning System Bot 2013-02-13 09:32:36 PST 
Comment on attachment 188098 [details]
Cleanup

Attachment 188098 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16537200
Comment 5 Early Warning System Bot 2013-02-13 09:33:07 PST 
Comment on attachment 188098 [details]
Cleanup

Attachment 188098 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16543211
Comment 6 EFL EWS Bot 2013-02-13 09:35:56 PST 
Comment on attachment 188098 [details]
Cleanup

Attachment 188098 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16514224
Comment 7 Peter Beverloo (cr-android ews) 2013-02-13 09:36:44 PST 
Comment on attachment 188098 [details]
Cleanup

Attachment 188098 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/16538177
Comment 8 WebKit Review Bot 2013-02-13 10:00:26 PST 
Comment on attachment 188098 [details]
Cleanup

Attachment 188098 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16454927
Comment 9 kov's GTK+ EWS bot 2013-02-13 10:04:03 PST 
Comment on attachment 188098 [details]
Cleanup

Attachment 188098 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/16536200
Comment 10 Ryosuke Niwa 2013-02-13 10:17:15 PST 
Created attachment 188113 [details]
Fixed build
Comment 11 Enrica Casucci 2013-02-13 11:50:29 PST 
Comment on attachment 188113 [details]
Fixed build

View in context: https://bugs.webkit.org/attachment.cgi?id=188113&action=review

Nicely done. Please address my comment before landing.

> Source/WebCore/editing/markup.cpp:650
> +    RefPtr<Range> updatedRangeRef = frame->editor()->avoidIntersectionWithDeleteButtonController(range);

There is now nothing that checks here if the frame is not NULL. If the check was needed before I think you should add it.
Comment 12 Ryosuke Niwa 2013-02-15 12:01:13 PST 
Committed r143030: <http://trac.webkit.org/changeset/143030>
Comment 13 Ryosuke Niwa 2013-02-15 13:53:25 PST 
Fixed one obvious error in my patch: http://trac.webkit.org/changeset/143048