Bug 54841

Summary:

Text overflow ellipsis wrong color when using webkit-text-fill-color

Product:

WebKit

Reporter:

Luke Alonso <luke>

Component:

Layout and Rendering

Assignee:

Rob Buis <rwlbuis>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dbates, dglazkov, eric, luke, ojan, rwlbuis, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.6

Attachments:

Description	Flags
A repro case	none
Patch	none
Archive of layout-test-results from ec2-cr-linux-04	none
Patch	none
Archive of layout-test-results from ec2-cr-linux-04	none
Patch	none
Archive of layout-test-results from ec2-cr-linux-03	none
Patch	none
Patch	none
Patch	none
Patch	hyatt: review+

Luke Alonso

Reported 2011-02-20 18:50:41 PST

Created attachment 83114 [details] A repro case The ellipsis shown when using 'text-overflow: ellipsis' appears to be the wrong color when the text color has been specified using webkit-text-fill-color. I imagine it's rendering the ellipsis using the standard 'color' style. A random, likely wrong, guess after a cursory inspection of the codebase, is that it involves the following in EllipsisBox::paint: Color textColor = style->visitedDependentColor(CSSPropertyColor); if (textColor != context->fillColor()) context->setFillColor(textColor, style->colorSpace()); A simple test case is attached. I've reproduced this in Safari 5.0.3 and webkit nightly 533.19.4, r78794.

Attachments
A repro case (772 bytes, text/html) 2011-02-20 18:50 PST, Luke Alonso	no flags	Details
Patch (1.56 KB, patch) 2012-06-08 19:01 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cr-linux-04 (742.19 KB, application/zip) 2012-06-09 00:41 PDT, WebKit Review Bot	no flags	Details
Patch (13.58 KB, patch) 2012-06-09 11:39 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cr-linux-04 (598.82 KB, application/zip) 2012-06-09 14:05 PDT, WebKit Review Bot	no flags	Details
Patch (13.55 KB, patch) 2012-06-11 13:28 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cr-linux-03 (1.42 MB, application/zip) 2012-06-11 15:05 PDT, WebKit Review Bot	no flags	Details
Patch (13.51 KB, patch) 2012-11-17 11:19 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (3.93 KB, patch) 2012-11-17 14:23 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (3.97 KB, patch) 2012-11-19 10:34 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (4.04 KB, patch) 2012-11-19 18:40 PST, Rob Buis	hyatt: review+	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Luke Alonso

Comment 1 2011-02-27 11:05:59 PST

Comment on attachment 83114 [details] A repro case ><html> ><body> ><div style="width: 225px; font-size: 24px; overflow: hidden; text-overflow: ellipsis; -webkit-text-fill-color: red; white-space: nowrap;"> >Wrong - This is a long string, which is clipped, but the ellipsis is the wrong color! ></div> ><div style="width: 225px; font-size: 24px; overflow: hidden; text-overflow: ellipsis; color: blue; white-space: nowrap;"> >Right - This is a long string, which is clipped, and the ellipsis is the right color! ></div ></body> ></html>

Rob Buis

Comment 2 2012-06-08 19:01:56 PDT

Created attachment 146679 [details] Patch

Rob Buis

Comment 3 2012-06-08 19:02:42 PDT

Comment on attachment 146679 [details] Patch Just testing for now.

WebKit Review Bot

Comment 4 2012-06-08 19:05:27 PDT

Attachment 146679 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

WebKit Review Bot

Comment 5 2012-06-09 00:40:58 PDT

Comment on attachment 146679 [details] Patch Attachment 146679 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12935203 New failing tests: fast/css/text-overflow-ellipsis-strict.html fast/css/text-overflow-ellipsis.html

WebKit Review Bot

Comment 6 2012-06-09 00:41:02 PDT

Created attachment 146692 [details] Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Rob Buis

Comment 7 2012-06-09 11:39:07 PDT

Created attachment 146711 [details] Patch

WebKit Review Bot

Comment 8 2012-06-09 14:05:24 PDT

Comment on attachment 146711 [details] Patch Attachment 146711 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12919820 New failing tests: fast/css/text-overflow-ellipsis-color.html

WebKit Review Bot

Comment 9 2012-06-09 14:05:28 PDT

Created attachment 146718 [details] Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Rob Buis

Comment 10 2012-06-11 13:28:40 PDT

Created attachment 146893 [details] Patch

WebKit Review Bot

Comment 11 2012-06-11 15:05:05 PDT

Comment on attachment 146893 [details] Patch Attachment 146893 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12948247 New failing tests: fast/css/text-overflow-ellipsis-color.html

WebKit Review Bot

Comment 12 2012-06-11 15:05:09 PDT

Created attachment 146924 [details] Archive of layout-test-results from ec2-cr-linux-03 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Rob Buis

Comment 13 2012-11-17 11:19:21 PST

Created attachment 174822 [details] Patch

Darin Adler

Comment 14 2012-11-17 13:07:33 PST

Comment on attachment 174822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174822&action=review > LayoutTests/ChangeLog:12 > + * platform/mac/fast/css/text-overflow-ellipsis-color-expected.png: Added. > + * platform/mac/fast/css/text-overflow-ellipsis-color-expected.txt: Added. This should be done with a reference test, not a Mac-only pixel test.

WebKit Review Bot

Comment 15 2012-11-17 14:13:46 PST

Comment on attachment 174822 [details] Patch Attachment 174822 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14868639 New failing tests: fast/css/text-overflow-ellipsis-color.html

Rob Buis

Comment 16 2012-11-17 14:23:17 PST

Created attachment 174828 [details] Patch

WebKit Review Bot

Comment 17 2012-11-17 15:18:42 PST

Comment on attachment 174828 [details] Patch Attachment 174828 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14875456 New failing tests: fast/css/text-overflow-ellipsis-color.html

Rob Buis

Comment 18 2012-11-19 10:34:48 PST

Created attachment 175003 [details] Patch

Rob Buis

Comment 19 2012-11-19 10:38:50 PST

(In reply to comment #18) > Created an attachment (id=175003) [details] > Patch I updated the patch to make it clear that part of the sentence should get clipped and replaced by ellipsis. If I am lucky cr-linux will stop complaining...

WebKit Review Bot

Comment 20 2012-11-19 12:27:20 PST

Comment on attachment 175003 [details] Patch Attachment 175003 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14906296 New failing tests: fast/css/text-overflow-ellipsis-color.html

Rob Buis

Comment 21 2012-11-19 18:40:47 PST

Created attachment 175110 [details] Patch

Rob Buis

Comment 22 2012-11-20 06:50:30 PST

(In reply to comment #21) > Created an attachment (id=175110) [details] > Patch It seems like Dirk's intuition was right, i.e. using Ahem, thanks!

Daniel Bates

Comment 23 2012-11-21 14:09:39 PST

I noticed that we also don't support -webkit-text-stroke, -webkit-text-stroke-color, and -webkit-text-stroke-width for an ellipsis.

Rob Buis

Comment 24 2012-11-21 14:14:36 PST

Hi Daniel, (In reply to comment #23) > I noticed that we also don't support -webkit-text-stroke, -webkit-text-stroke-color, and -webkit-text-stroke-width for an ellipsis. Right, but I assume that it is not needed for the ellipsis/dots. I may check against other implementations later...

Daniel Bates

Comment 25 2012-11-21 14:24:24 PST

(In reply to comment #24) > Hi Daniel, > > (In reply to comment #23) > > I noticed that we also don't support -webkit-text-stroke, -webkit-text-stroke-color, and -webkit-text-stroke-width for an ellipsis. > > Right, but I assume that it is not needed for the ellipsis/dots. Can you elaborate on your reasoning on why it's unnecessary to support stroke styles for an ellipsis? > I may check against other implementations later... As far as I can tell, -webkit-text-{fill-color, stroke, stroke-color, width} are proprietary WebKit extensions.

Daniel Bates

Comment 26 2012-11-21 14:25:59 PST

(In reply to comment #25) > [...] > As far as I can tell, -webkit-text-{fill-color, stroke, stroke-color, width} are proprietary WebKit extensions. s/width/stroke-width

Rob Buis

Comment 27 2012-11-21 14:56:10 PST

(In reply to comment #25) > (In reply to comment #24) > > Hi Daniel, > > > > (In reply to comment #23) > > > I noticed that we also don't support -webkit-text-stroke, -webkit-text-stroke-color, and -webkit-text-stroke-width for an ellipsis. > > > > Right, but I assume that it is not needed for the ellipsis/dots. > > Can you elaborate on your reasoning on why it's unnecessary to support stroke styles for an ellipsis? This is my personal opinion, based on how I imagine it would look. Like described below I can't turn this on for FF, I tried -moz-text-stroke-color and -moz-text-stroke-width. > > I may check against other implementations later... > > As far as I can tell, -webkit-text-{fill-color, stroke, stroke-color, width} are proprietary WebKit extensions. Yeah, from experiments there is nothing equivalent in FF, so that does not help. Given all that, and that I can't see anything about this in the spec, I think it is best to just stick to this patch, and if needed open a bug for the -webkit-text-stroke thing.

Dave Hyatt

Comment 28 2013-02-28 12:31:25 PST

Comment on attachment 175110 [details] Patch r=me

Rob Buis

Comment 29 2013-03-02 13:13:32 PST

Committed r144542: <http://trac.webkit.org/changeset/144542>

Note You need to log in before you can comment on or make changes to this bug.