Bug 35185

Summary:

Move text caret drawing code into RenderTheme

Product:

WebKit

Reporter:

Daniel Bates <dbates>

Component:

HTML Editing

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

hyatt, manyoso, mitz, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	hyatt: review-
Patch	none

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	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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

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.