Bug 35185 - Move text caret drawing code into RenderTheme
Summary: Move text caret drawing code into RenderTheme
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-19 16:17 PST by Daniel Bates
Modified: 2010-06-11 10:01 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.50 KB, patch)
2010-02-19 16:23 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (7.66 KB, patch)
2010-03-08 10:46 PST, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (7.66 KB, patch)
2010-03-08 10:57 PST, Daniel Bates
hyatt: review-
Details | Formatted Diff | Diff
Patch (7.86 KB, patch)
2010-03-10 11:13 PST, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2010-02-19 16:17:35 PST
We should move the text caret drawing code in SelectionController::paintCaret into RenderTheme so that platforms can optionally draw the caret with respect to their theme.
Comment 1 Daniel Bates 2010-02-19 16:23:52 PST
Created attachment 49110 [details]
Patch

No functionality was changes so no tests are included in this patch.
Comment 2 Eric Seidel (no email) 2010-02-22 14:22:53 PST
Comment on attachment 49110 [details]
Patch

What would that mean?  Do RIM devices not have a caret?
Comment 3 Daniel Bates 2010-02-24 22:34:52 PST
(In reply to comment #2)
> (From update of attachment 49110 [details])
> What would that mean?  Do RIM devices not have a caret?

RIM devices have a caret. It is painted using the OS theme colors.
Comment 4 Daniel Bates 2010-02-24 22:35:32 PST
Comment on attachment 49110 [details]
Patch

Looking into this proposal some more.
Comment 5 Daniel Bates 2010-03-08 10:46:37 PST
Created attachment 50233 [details]
Patch

Updated patch following the landing of the patch for bug #34819 in changeset 55669 <http://trac.webkit.org/changeset/55669>.

Also, moved repaintCaret code into a similar RenderTheme method.
Comment 6 WebKit Review Bot 2010-03-08 10:53:56 PST
Attachment 50233 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/editing/SelectionController.cpp:1022:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Daniel Bates 2010-03-08 10:57:10 PST
Created attachment 50234 [details]
Patch

Fixed style error.
Comment 8 Dave Hyatt 2010-03-09 17:32:57 PST
Comment on attachment 50234 [details]
Patch

Is defaultTheme() really correct?  I thought Qt had the notion of Page-specific themes.
Comment 9 Daniel Bates 2010-03-10 11:13:07 PST
Created attachment 50417 [details]
Patch

Updated patch to use the page-specific theme.
Comment 10 Dave Hyatt 2010-03-10 11:45:43 PST
Comment on attachment 50417 [details]
Patch

So I get why you might want the theme to paint the caret, but what is the point of a repaint method being in the theme?  I don't understand this part.
Comment 11 Daniel Bates 2010-03-10 12:04:16 PST
Comment on attachment 50417 [details]
Patch

Will look into alternative to repaintCaret method.