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 treegraph
 
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

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