Bug 109550

Summary: DeleteButtonController::enable and disable should be called via a RAII object
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, darin, dglazkov, enrica, gtk-ews, leviw, mifenton, peter+ews, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 109549    
Bug Blocks:    
Attachments:
Description Flags
Cleanup
none
Cleanup
none
Fixed build enrica: review+

Ryosuke Niwa
Reported 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.
Attachments
Cleanup (8.56 KB, patch)
2013-02-13 09:25 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Cleanup (8.08 KB, patch)
2013-02-13 09:25 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fixed build (8.11 KB, patch)
2013-02-13 10:17 PST, Ryosuke Niwa 		enrica: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2013-02-13 09:25:15 PST
Created attachment 188097 [details] Cleanup
Ryosuke Niwa
Comment 2 2013-02-13 09:25:48 PST
Created attachment 188098 [details] Cleanup
WebKit Review Bot
Comment 3 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
Early Warning System Bot
Comment 4 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
Early Warning System Bot
Comment 5 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
EFL EWS Bot
Comment 6 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
Peter Beverloo (cr-android ews)
Comment 7 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
WebKit Review Bot
Comment 8 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
kov's GTK+ EWS bot
Comment 9 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
Ryosuke Niwa
Comment 10 2013-02-13 10:17:15 PST
Created attachment 188113 [details] Fixed build
Enrica Casucci
Comment 11 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.
Ryosuke Niwa
Comment 12 2013-02-15 12:01:13 PST
Committed r143030: <http://trac.webkit.org/changeset/143030>
Ryosuke Niwa
Comment 13 2013-02-15 13:53:25 PST
Fixed one obvious error in my patch: http://trac.webkit.org/changeset/143048
Note You need to log in before you can comment on or make changes to this bug.