Bug 127986

Summary: Remove inline member functions of FrameSelection that access m_selection
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, benjamin, darin, enrica, kling, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 127832    
Attachments:
Description Flags
Cleanup
none
Fixes GTK+ and EFL builds
none
Another build fix
none
One more build fix for EFL & GTK+
none
Fix EFL build enrica: review+

Ryosuke Niwa
Reported 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().
Attachments
Cleanup (72.45 KB, patch)
2014-01-30 23:48 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fixes GTK+ and EFL builds (72.38 KB, patch)
2014-01-31 00:22 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Another build fix (73.53 KB, patch)
2014-01-31 00:39 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
One more build fix for EFL & GTK+ (75.01 KB, patch)
2014-01-31 14:39 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fix EFL build (75.02 KB, patch)
2014-01-31 15:45 PST, Ryosuke Niwa 		enrica: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2014-01-30 23:48:26 PST
Created attachment 222798 [details] Cleanup
Ryosuke Niwa
Comment 2 2014-01-31 00:22:48 PST
Created attachment 222802 [details] Fixes GTK+ and EFL builds
Ryosuke Niwa
Comment 3 2014-01-31 00:39:15 PST
Created attachment 222805 [details] Another build fix
Sergio Villar Senin
Comment 4 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?
Ryosuke Niwa
Comment 5 2014-01-31 14:39:55 PST
Created attachment 222855 [details] One more build fix for EFL & GTK+
Ryosuke Niwa
Comment 6 2014-01-31 15:45:00 PST
Created attachment 222861 [details] Fix EFL build
Enrica Casucci
Comment 7 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.
Ryosuke Niwa
Comment 8 2014-01-31 20:06:05 PST
Committed r163232: <http://trac.webkit.org/changeset/163232>
Ryosuke Niwa
Comment 9 2014-01-31 20:53:45 PST
Debug tests fix landed in http://trac.webkit.org/changeset/163233.
Note You need to log in before you can comment on or make changes to this bug.