Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-text-decor] Default color of text-shadow #942

Closed
jfkthame opened this issue Jan 16, 2017 · 29 comments
Closed

[css-text-decor] Default color of text-shadow #942

jfkthame opened this issue Jan 16, 2017 · 29 comments

Comments

@jfkthame
Copy link
Contributor

https://drafts.csswg.org/css-text-decor-3/#text-shadow-property says that:

If the color of the shadow is not specified, the shadow has the resulting color of the ink that it shadows.

This seems to imply, for instance, that a colored emoji glyph would by default (i.e. no explicit shadow color is specified) cast a multi-colored shadow, as its ink is multi-colored. But ISTM that a "shadow" is inherently monochrome, regardless of the color(s) of the thing being shadowed.

Perhaps this text was written with text decorations in mind, given that text-decoration-color can give different colors to underline, overline, etc.; so the "ink" of a decoration may have a different color than the "ink" of the associated text, and the intent may have been to allow different-colored decorations to cast corresponding different-colored shadows.

But this still seems wrong to me. In the case of text with a colored underline (perhaps even multiple, differently-colored decoration lines), I think the natural interpretation of text-shadow is that it causes the complete (decorated) text rendering to cast a single, monochromatic shadow, and not that it causes the (bare) text to cast a shadow that then has various-colored decorations applied to it.

Also, compare box-shadow, which says:

If the color is absent, the used color is taken from the ‘color’ property.

So a multi-colored <img>, for example, does not magically cast a multi-colored shadow. Neither should multi-colored decorated text, IMO, cast a multi-colored shadow.

Therefore, I suggest the text-shadow spec should be changed to be like box-shadow in this respect.

@upsuper
Copy link
Member

upsuper commented Jan 16, 2017

cc myself

@kuoe0
Copy link

kuoe0 commented Jan 18, 2017

CC myself.

@FremyCompany
Copy link
Contributor

image image image
https://jsfiddle.net/bqzwj1ke/ (Edge, Firefox, Chrome)

@FremyCompany
Copy link
Contributor

@upsuper
Copy link
Member

upsuper commented Feb 5, 2017

I guess it means we do want to change the spec to make the whole shadow use currentcolor by default rather than the "color of the ink".

@kojiishi kojiishi changed the title [css-text] Default color of text-shadow [css-text-decor] Default color of text-shadow Feb 6, 2017
@kojiishi kojiishi added the css-text-decor-3 Current Work label Feb 6, 2017
@kojiishi
Copy link
Contributor

kojiishi commented Feb 6, 2017

So the proposals from @jfkthame and @upsuper are the same, correct? Sounds reasonable to me IIUC.

@FremyCompany's test is interesting...for shadows of decorations, should we honor the used color of color property, or propagated color? Anyone have any preferences?

@upsuper
Copy link
Member

upsuper commented Feb 6, 2017

The motivation of this proposal was that, having different color in a blur shadow is hard to implement (takes time but doesn't seem to be useful), so I think the proposal is to make the shadow of decoration lines always use the same color as the text.

More formally, the proposal should be something like, when color is omitted, it is treated as if currentcolor is specified.

@FremyCompany
Copy link
Contributor

Then there is also the question of chrome repainting for each text decoration instead of one for the complete geometry. We don't have this issue in Edge as a result of how we do underlining (there can be only one, but its color will vary depending on closest text-decoration:underline parent).

@upsuper
Copy link
Member

upsuper commented Feb 6, 2017

If we treat omitted <color> as currentcolor, then I don't think there is ambiguity anymore. Everything from the same text-shadow would have the same color.

@kojiishi
Copy link
Contributor

kojiishi commented Feb 7, 2017

@FremyCompany I'm failing to understand... Can you explain a bit more?

@FremyCompany
Copy link
Contributor

@kojiishi Look at the underline under "bcd" in Chrome in case 1 vs case 2. You will see that the underline in case 1 is "darker" because it seems to be drawn twice (once for the parent "a" underline, and once for the child "span" underline).

@kojiishi
Copy link
Contributor

kojiishi commented Feb 8, 2017

@FremyCompany oh I see, thank you. Yes, we draw all of nested decorations in order to support different style/position etc., ex. http://jsbin.com/kikuju

@FremyCompany
Copy link
Contributor

@kojiishi I see. But that means that you render using full-color blur, since the underlines can overlap and compose to darker colors. In Edge we try to use the geometry not redraw the text (the same thing we do for background-clip:text, actually).

Also, here is a test case that shows how non-interoperable we are:

image image image
https://jsfiddle.net/k53Ld4g3/ (Edge, Firefox, Chrome)

As I understand, this thread seems to be about standardizing Edge's behavior, right?

@upsuper
Copy link
Member

upsuper commented Feb 9, 2017

Isn't it about standardizing Chrome's behavior? Chrome uses the currentcolor for shadow of decoration lines. If there is any blur, you would see that Firefox would have the same behavior, and we want to align our behavior between blur and non-blur cases. The easiest way so far seems to be aligning the non-blur case to the blur case for us.

@kojiishi
Copy link
Contributor

kojiishi commented Feb 9, 2017

Do we agree that "rendering nested decorations" and "color of shadow, including shadow of decorations" are separate issues? Related, but I suppose we can repro the former without shadows, using alpha for decoration color.

For shadow, Edge looks most reasonable to me, but I might need to check with other members in our team.

For nested decorations, I haven't determined my thoughts well enough yet. This might worth a separate issue if we were that non-interoperable.

@upsuper
Copy link
Member

upsuper commented Feb 9, 2017

Do we agree that "rendering nested decorations" and "color of shadow, including shadow of decorations" are separate issues? Related, but I suppose we can repro the former without shadows, using alpha for decoration color.

Isn't nested decoration supposed to exist? I think that is a natural consequence of propagation style text-decoration which Edge probably just haven't implemented yet.

I believe this issue is simply about the color of shadow of decorations.

For shadow, Edge looks most reasonable to me, but I might need to check with other members in our team.

That means we give text shadow a default color of grey or semi-transparent black. That might be fine as well.

I don't know why text-shadow currently uses color of "ink" by default. But I would probably prefer we use currentcolor as the default, to align with the behavior of box-shadow.

@kojiishi
Copy link
Contributor

kojiishi commented Feb 9, 2017

I don't know why text-shadow currently uses color of "ink" by default

I think it meant currentColor, just written before color Emoji was invented. @fantasai could you chime in?

@upsuper
Copy link
Member

upsuper commented Feb 9, 2017

I think it meant currentColor, just written before color Emoji was invented.

But decoration lines are defined in the same spec, so color of "ink" is already ambiguous in the spec itself regardless of emoji.

@FremyCompany
Copy link
Contributor

FremyCompany commented Feb 9, 2017

@upsuper I think you had a too quick look at the test case. Since it is defined on the body, the shadow uses the currentColor of the body element which I set to some gray. If I had set two shadows instead (one of the "a" and one on the "b") then Chrome's behavior would be correct (and would match Edge in that case).

@tantek tantek added the Agenda+ label Feb 14, 2017
@upsuper
Copy link
Member

upsuper commented Feb 14, 2017

@FremyCompany Yeah, I probably look too quick, sorry.

But in that testcase, Chrome's behavior still matches what I expect from the spec with my proposed change, because text-shadow inherits, and currentcolor is a computed-value time value, which means the shadow should use the value of color property of the current text rather than color of the element with text-shadow specified. Whether it is desired is another question, though...

@FremyCompany
Copy link
Contributor

FremyCompany commented Feb 15, 2017

Oh well, currentColor is an annoying thing sometimes... I guess you are right, this is the per-spec behavior. I prefer either having everything the same color or having the underlines respect their original color, a behavior where text preserves its color but not the underline is confusing.

Anyway, I ended up on this issue for another reason: I was wondering what browsers do in the case of color emojis. I like Edge's behavior better but I guess it only works because on Windows we maintain strict parity between color and non-color emojis:

https://jsfiddle.net/k53Ld4g3/3/
image image

Any opinion on that?

Also, @upsuper: I found out that in Firefox the text shadow disappears on selection, is that by design?

@kuoe0
Copy link

kuoe0 commented Feb 16, 2017

@FremyCompany Can you provide the test case about the disappearance of text-shadow on selection? I can't reproduce with https://jsfiddle.net/pd3zoymq/.

@FremyCompany
Copy link
Contributor

@kuoe0 The testcase you just mentioned reproduces on my pc, as all others do too. Both Firefox 53.0a2 (2017-02-10) and 51.0.1 are affected on my Windows 10 machine.
image

@upsuper
Copy link
Member

upsuper commented Feb 18, 2017

The testcase you just mentioned reproduces on my pc, as all others do too. Both Firefox 53.0a2 (2017-02-10) and 51.0.1 are affected on my Windows 10 machine.

Oh, that's... definitely a bug of Firefox. It is because we change the text color when rendering selected text. That theoretically shouldn't affect the color used to render the shadow.

@dbaron
Copy link
Member

dbaron commented Feb 18, 2017

We've made a bunch of changes in the past to how text-shadow and selection interact; I think for more "normal" text shadows, leaving the original color during selection looks pretty bad.

@tabatkins
Copy link
Member

Resolution from today's call is to simplify the model to tie the shadow to the "current text color". We didn't resolve on precise text, instead deferring to this issue to hash it out.

I believe that just having it default to currentcolor works. For example, in

<!DOCTYPE html>
<div><span class=one>foo</span><span class=two>bar</span></div>
<style>
div { text-shadow: 2px 2px 2px; }
.one { color: red; }
.two { color: black; }
</style>

Chrome, at least, shows the "foo" with a red shadow and the "bar" with a black shadow. This is consistent with inheriting a currentcolor value.

And I think this handles selection properly too - the currentcolor inherits all the way down to the text nodes, which have their color changed by ::selection, so the shadow will change color as well.

Does this sound good?

@FremyCompany
Copy link
Contributor

If the div has an underline, does that underline end up being of the color of the text of the div (black)? That should be what happens, right? If so, I think the proposed text lgtm.

@fantasai
Copy link
Collaborator

fantasai commented Apr 6, 2017

@fantasai
Copy link
Collaborator

fantasai commented Apr 6, 2017

Sounds great. I'm calling this issue closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants