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

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.