Bug 45508 - Move functions from Frame to SelectionController as planned
Summary: Move functions from Frame to SelectionController as planned
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
: 23431 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-09 18:00 PDT by Darin Adler
Modified: 2012-05-30 00:50 PDT (History)
4 users (show)

See Also:


Attachments
Patch (82.65 KB, patch)
2010-09-09 18:23 PDT, Darin Adler
abarth: review+
abarth: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2010-09-09 18:00:15 PDT
Move functions from Frame to SelectionController as planned
Comment 1 Darin Adler 2010-09-09 18:23:32 PDT
Created attachment 67130 [details]
Patch
Comment 2 WebKit Review Bot 2010-09-09 20:09:01 PDT
Attachment 67130 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3974365
Comment 3 Adam Barth 2010-09-09 21:27:33 PDT
Comment on attachment 67130 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67130&action=prettypatch

Please try not to break the chromium build when landing.  Thanks!

> WebKit/chromium/src/WebFrameImpl.cpp:1716
> +        return IntRect(frame()->selection()->selectionBounds(false));
Not just selection()->bounds(false) ?

> WebKit/mac/WebView/WebHTMLView.mm:6052
> +        coreFrame->selection()->selectionTextRects(list, SelectionController::RespectTransforms);
selection()->selectionTextRects => selection()->textRects ?
Comment 4 Darin Adler 2010-09-10 08:13:17 PDT
Sure, we could rename these functions now that they are on the SelectionController, but I think it’s probably best to do that in a separate pass.

I’ll think about doing it now, though.
Comment 5 Adam Barth 2010-09-10 10:03:20 PDT
ok.  I just noticed you renamed granularity, which seemed like a good idea.
Comment 6 Darin Adler 2010-09-10 10:13:14 PDT
(In reply to comment #5)
> I just noticed you renamed granularity, which seemed like a good idea.

That case was different; believe it or not I didn’t rename things. Frame::selectionGranularity() was a function with the body "return selection()->granularity()", and I chose to delete that rather than moving it to SelectionController.

One problem with renaming is that SelectionController is accessed with a call named selection() yet represents more than just the selection; the broader concept of selection control. So in some cases we might need to keep selection in the names of selection controller functions.

But despite that it does seem likely we can remove the word selection from virtually all SelectionController function names. Just not sure if I should do it now as part of this patch.
Comment 7 Darin Adler 2010-09-10 15:07:09 PDT
Committed r67238: <http://trac.webkit.org/changeset/67238>
Comment 8 Ryosuke Niwa 2012-05-30 00:50:58 PDT
*** Bug 23431 has been marked as a duplicate of this bug. ***