Bug 117493 - Caret should respect text background color
Summary: Caret should respect text background color
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: Joone Hur
URL:
Keywords: BlinkMergeCandidate
Depends on:
Blocks: 44862
  Show dependency treegraph
 
Reported: 2013-06-10 23:01 PDT by Ryosuke Niwa
Modified: 2013-07-22 12:49 PDT (History)
6 users (show)

See Also:


Attachments
Patch (5.57 KB, patch)
2013-06-26 01:00 PDT, Joone Hur
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (519.87 KB, application/zip)
2013-06-26 05:41 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (521.31 KB, application/zip)
2013-06-26 08:11 PDT, Build Bot
no flags Details
Archive of layout-test-results from APPLE-EWS-6 for win-future (842.70 KB, application/zip)
2013-06-26 09:14 PDT, Build Bot
no flags Details
Patch (19.80 KB, patch)
2013-06-28 01:44 PDT, Joone Hur
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (613.90 KB, application/zip)
2013-06-28 02:48 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 (666.50 KB, application/zip)
2013-06-28 03:24 PDT, Build Bot
no flags Details
Archive of layout-test-results from APPLE-EWS-3 for win-future (858.32 KB, application/zip)
2013-06-29 08:13 PDT, Build Bot
no flags Details
Archive of layout-test-results from APPLE-EWS-2 for win-future (858.31 KB, application/zip)
2013-06-29 10:16 PDT, Build Bot
no flags Details
Patch (20.59 KB, patch)
2013-07-09 23:06 PDT, Joone Hur
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2013-06-10 23:01:07 PDT
Consider merging https://chromium.googlesource.com/chromium/blink/+/3f7fa25f27ce8e067136626f6fbef2e9341aab2c.

diff --git a/Source/core/editing/FrameSelection.cpp b/Source/core/editing/FrameSelection.cpp
index 5a5f03c..229f473 100644
--- a/Source/core/editing/FrameSelection.cpp
+++ b/Source/core/editing/FrameSelection.cpp
@@ -1458,7 +1458,13 @@
         return;
 
     Color caretColor = Color::black;
-    Element* element = node->rootEditableElement();
+
+    Element* element;
+    if (node->isElementNode())
+        element = toElement(node);
+    else
+        element = node->parentElement();
+
     if (element && element->renderer())
         caretColor = element->renderer()->style()->visitedDependentColor(CSSPropertyColor);
Comment 1 Joone Hur 2013-06-17 21:46:26 PDT
Hi Ryosoke,

I made this patch so I will prepare the patch for WebKit.

Thanks,
Joone
Comment 2 Joone Hur 2013-06-17 21:50:48 PDT
(In reply to comment #1)
> Hi Ryosoke,

Ryosuke, I'm sorry that I misspelled your name. :-)
Comment 3 Joone Hur 2013-06-26 01:00:43 PDT
Created attachment 205460 [details]
Patch
Comment 4 Build Bot 2013-06-26 05:41:53 PDT
Comment on attachment 205460 [details]
Patch

Attachment 205460 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/912792

New failing tests:
editing/caret/caret-color.html
Comment 5 Build Bot 2013-06-26 05:41:54 PDT
Created attachment 205479 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 6 Build Bot 2013-06-26 08:11:32 PDT
Comment on attachment 205460 [details]
Patch

Attachment 205460 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/930463

New failing tests:
editing/caret/caret-color.html
Comment 7 Build Bot 2013-06-26 08:11:34 PDT
Created attachment 205493 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 8 Build Bot 2013-06-26 09:14:19 PDT
Comment on attachment 205460 [details]
Patch

Attachment 205460 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/967308

New failing tests:
fast/media/mq-transform-03.html
fast/forms/search-event-delay.html
fast/media/mq-transform-02.html
editing/caret/caret-color.html
platform/win/accessibility/multiple-select-element-role.html
Comment 9 Build Bot 2013-06-26 09:14:21 PDT
Created attachment 205502 [details]
Archive of layout-test-results from APPLE-EWS-6 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-6  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 10 Ryosuke Niwa 2013-06-26 09:50:26 PDT
Comment on attachment 205460 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Test: editing/caret/caret-color.html

This line should appear after the long description proceeded and succeeded by a single blank line.

> Source/WebCore/ChangeLog:13
> +

i.e. here.

> Source/WebCore/editing/FrameSelection.cpp:1477
> +    Element* element;
> +    if (node->isElementNode())
> +        element = toElement(node);
> +    else
> +        element = node->parentElement();

I would have preferred to use tertiary expression here as in:
Element* element = node->isElementNode() = toElement(node) : node->parentElement();

> LayoutTests/ChangeLog:14
> +        * editing/caret/caret-color-expected.txt: Added.
> +        * editing/caret/caret-color.html: Added.

We need .png file. Use run-webkit-tests --pixel.

> LayoutTests/editing/caret/caret-color.html:1
> +<html> 

Missing DOCTYPE.

> LayoutTests/editing/caret/caret-color.html:10
> +<style>
> +.editing { 
> +    border: 2px solid red; 
> +    padding: 12px; 
> +    font-size: 24px; 
> +}
> +</style>

This is really for old editing tests. We don't need these styles anymore.
Comment 11 Joone Hur 2013-06-27 23:32:29 PDT
(In reply to comment #10)
> (From update of attachment 205460 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205460&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Test: editing/caret/caret-color.html
> 
> This line should appear after the long description proceeded and succeeded by a single blank line.
> 
> > Source/WebCore/ChangeLog:13
> > +
> 
> i.e. here.

Done.

> 
> > Source/WebCore/editing/FrameSelection.cpp:1477
> > +    Element* element;
> > +    if (node->isElementNode())
> > +        element = toElement(node);
> > +    else
> > +        element = node->parentElement();
> 
> I would have preferred to use tertiary expression here as in:
> Element* element = node->isElementNode() = toElement(node) : node->parentElement();

Done.

> 
> > LayoutTests/ChangeLog:14
> > +        * editing/caret/caret-color-expected.txt: Added.
> > +        * editing/caret/caret-color.html: Added.
> 
> We need .png file. Use run-webkit-tests --pixel.

Done, but I am just able to add a png file for the EFL port.

> 
> > LayoutTests/editing/caret/caret-color.html:1
> > +<html> 
> 
> Missing DOCTYPE.
> 
> > LayoutTests/editing/caret/caret-color.html:10
> > +<style>
> > +.editing { 
> > +    border: 2px solid red; 
> > +    padding: 12px; 
> > +    font-size: 24px; 
> > +}
> > +</style>
> 
> This is really for old editing tests. We don't need these styles anymore.

Done.

I will upload an updated patch soon.
Thank you for the review.
Comment 12 Joone Hur 2013-06-28 01:44:25 PDT
Created attachment 205678 [details]
Patch
Comment 13 Build Bot 2013-06-28 02:48:50 PDT
Comment on attachment 205678 [details]
Patch

Attachment 205678 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/866481

New failing tests:
editing/caret/caret-color.html
Comment 14 Build Bot 2013-06-28 02:48:52 PDT
Created attachment 205685 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.3
Comment 15 Build Bot 2013-06-28 03:24:39 PDT
Comment on attachment 205678 [details]
Patch

Attachment 205678 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/998220

New failing tests:
editing/caret/caret-color.html
Comment 16 Build Bot 2013-06-28 03:24:41 PDT
Created attachment 205692 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 17 Build Bot 2013-06-29 08:13:32 PDT
Comment on attachment 205678 [details]
Patch

Attachment 205678 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/943665

New failing tests:
editing/caret/caret-color.html
Comment 18 Build Bot 2013-06-29 08:13:35 PDT
Created attachment 205768 [details]
Archive of layout-test-results from APPLE-EWS-3 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-3  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 19 Build Bot 2013-06-29 10:15:58 PDT
Comment on attachment 205678 [details]
Patch

Attachment 205678 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/866668

New failing tests:
fast/frames/seamless/seamless-nested-crash.html
fast/js/global-constructors-attributes-shared-worker.html
fast/forms/select/popup-closes-on-blur.html
fast/media/mq-transform-03.html
editing/selection/user-select-all-image-with-single-click.html
fast/forms/search-event-delay.html
fast/js/global-constructors-attributes-dedicated-worker.html
editing/selection/user-select-all-with-single-click.html
fast/media/mq-transform-02.html
editing/caret/caret-color.html
platform/win/accessibility/multiple-select-element-role.html
Comment 20 Build Bot 2013-06-29 10:16:00 PDT
Created attachment 205769 [details]
Archive of layout-test-results from APPLE-EWS-2 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-2  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 21 Joone Hur 2013-07-09 23:06:15 PDT
Created attachment 206369 [details]
Patch

This patch allows to skip the caret color test case on the Mac port
Comment 22 WebKit Commit Bot 2013-07-13 23:03:37 PDT
Comment on attachment 206369 [details]
Patch

Clearing flags on attachment: 206369

Committed r152612: <http://trac.webkit.org/changeset/152612>
Comment 23 WebKit Commit Bot 2013-07-13 23:03:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 Benjamin Poulain 2013-07-14 23:35:05 PDT
Don't forget to rebaseline.
Comment 25 Joone Hur 2013-07-22 12:49:56 PDT
(In reply to comment #24)
> Don't forget to rebaseline.

http://trac.webkit.org/changeset/152987

Thanks!