Bug 127986 - Remove inline member functions of FrameSelection that access m_selection
Summary: Remove inline member functions of FrameSelection that access m_selection
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:
Blocks: 127832
  Show dependency tree / graph
 
Reported: 2014-01-30 23:24 PST by Ryosuke Niwa
Modified: 2014-01-31 20:53 PST (History)
6 users (show)

See Also:

Attachments
Cleanup (72.45 KB, patch)
2014-01-30 23:48 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Fixes GTK+ and EFL builds (72.38 KB, patch)
2014-01-31 00:22 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Another build fix (73.53 KB, patch)
2014-01-31 00:39 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
One more build fix for EFL & GTK+ (75.01 KB, patch)
2014-01-31 14:39 PST, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Fix EFL build (75.02 KB, patch)
2014-01-31 15:45 PST, Ryosuke Niwa 		enrica: review+
Details | Formatted Diff | Diff
Show Obsolete (4) 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 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 1 Ryosuke Niwa 2014-01-30 23:48:26 PST 
Created attachment 222798 [details]
Cleanup
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 6 Ryosuke Niwa 2014-01-31 15:45:00 PST 
Created attachment 222861 [details]
Fix EFL build
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>
Comment 9 Ryosuke Niwa 2014-01-31 20:53:45 PST 
Debug tests fix landed in http://trac.webkit.org/changeset/163233.