|Summary:||Remove inline member functions of FrameSelection that access m_selection|
|Product:||WebKit||Reporter:||Ryosuke Niwa <rniwa>|
|Component:||HTML Editing||Assignee:||Ryosuke Niwa <rniwa>|
|Severity:||Normal||CC:||barraclough, benjamin, darin, enrica, kling, sam|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Ryosuke Niwa 2014-01-30 23:24:30 PST
In order to resolve the bug 127832, we need all call sites of FrameSelection's member functions that depend on m_selection to decide whether they want the invalidated selection that's exposed to JS or that the validated selection the editing & rendering code will use. In order to simplify this transition, all such access to m_selection should be done though FrameSelection::selection().
Comment 2 Ryosuke Niwa 2014-01-31 00:22:48 PST
Created attachment 222802 [details] Fixes GTK+ and EFL builds
Comment 3 Ryosuke Niwa 2014-01-31 00:39:15 PST
Created attachment 222805 [details] Another build fix
Comment 4 Sergio Villar Senin 2014-01-31 03:50:08 PST
Comment on attachment 222805 [details] Another build fix View in context: https://bugs.webkit.org/attachment.cgi?id=222805&action=review Great to see how the FrameSelection interface gets smaller. Patch looks awesome to me. Just a minor nit, DumpRenderTreeSupportGtk also accesses rootEditableElement() so you'd have to add a similar fix to the one you did for EFL. > Source/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp:219 > + const VisibleSelection& selection = frame.selection(); This should be frame.selection().selection() right?
Comment 5 Ryosuke Niwa 2014-01-31 14:39:55 PST
Created attachment 222855 [details] One more build fix for EFL & GTK+
Comment 7 Enrica Casucci 2014-01-31 16:37:06 PST
Comment on attachment 222861 [details] Fix EFL build View in context: https://bugs.webkit.org/attachment.cgi?id=222861&action=review Looks good to me. Please fix FrameSelection.h before landing otherwise it will break iOS builds. > Source/WebCore/editing/FrameSelection.h:343 > + bool m_updateAppearanceEnabled : 1;DragController.cpp This must be a typo. I'm sure it doesn't build like this for iOS :-) > Source/WebCore/page/DragController.cpp:816 > + const VisibleSelection& srcSelection = src.selection().selection(); I would call it sourceSelection.
Comment 8 Ryosuke Niwa 2014-01-31 20:06:05 PST
Committed r163232: <http://trac.webkit.org/changeset/163232>