NEW 35185
Move text caret drawing code into RenderTheme
https://bugs.webkit.org/show_bug.cgi?id=35185
Summary Move text caret drawing code into RenderTheme
Daniel Bates
Reported 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.
Attachments
Patch (3.50 KB, patch)
2010-02-19 16:23 PST, Daniel Bates
no flags
Patch (7.66 KB, patch)
2010-03-08 10:46 PST, Daniel Bates
no flags
Patch (7.66 KB, patch)
2010-03-08 10:57 PST, Daniel Bates
hyatt: review-
Patch (7.86 KB, patch)
2010-03-10 11:13 PST, Daniel Bates
no flags
Daniel Bates
Comment 1 2010-02-19 16:23:52 PST
Created attachment 49110 [details] Patch No functionality was changes so no tests are included in this patch.
Eric Seidel (no email)
Comment 2 2010-02-22 14:22:53 PST
Comment on attachment 49110 [details] Patch What would that mean? Do RIM devices not have a caret?
Daniel Bates
Comment 3 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.
Daniel Bates
Comment 4 2010-02-24 22:35:32 PST
Comment on attachment 49110 [details] Patch Looking into this proposal some more.
Daniel Bates
Comment 5 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.
WebKit Review Bot
Comment 6 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.
Daniel Bates
Comment 7 2010-03-08 10:57:10 PST
Created attachment 50234 [details] Patch Fixed style error.
Dave Hyatt
Comment 8 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.
Daniel Bates
Comment 9 2010-03-10 11:13:07 PST
Created attachment 50417 [details] Patch Updated patch to use the page-specific theme.
Dave Hyatt
Comment 10 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.
Daniel Bates
Comment 11 2010-03-10 12:04:16 PST
Comment on attachment 50417 [details] Patch Will look into alternative to repaintCaret method.
Note You need to log in before you can comment on or make changes to this bug.