Bug 18459 – text-shadow is not implemented

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Benjamin Meyer]

http://www.css3.info/preview/text-shadow/

or goto http://goldoptimusprime.com/ and the title "Transformers Gold Masterpiece Optimus Prime" should have a text shadow

[Jonathon Jongsma (jonner)]

Just thought I'd leave a note that I'm currently working on this

[Jonathon Jongsma (jonner)]

Created attachment 21843 [details]
Add text-shadow to the Qt port

Here's a potential Fix.  I also noticed that there's a commented-out function GraphicsContext::drawTextShadow() that seems to be out-of-date.  I assume this was written when text-shadows were done differently inside WebCore (before it supported multiple text-shadows perhaps?).  Anyway, my patch seems to roughly match what other ports do for text shadows.  I didn't remove the commented-out function yet since it seems to have a blur implementation and my patch doesn't support blurring yet.

[Simon Hausmann]

In principle I agree with the patch :). I think however it would be nice to have a hasShadow() function in GraphicsContext that quickly determines whether a shadow is needed. Right now the condition is duplicated in this patch in some places as well as in the cairo GraphicsContext. What do you think?

Also a port of the old blurring code would be nice, as well as making setPlatformShadow empty, since all the information is now also stored in the common state in GraphicsContext::setShadow.

[Jonathon Jongsma (jonner)]

Yes, I agree that a hasShadow() function would be useful.  I'll look into that.

Regarding the blur code -- I was planning on landing a plain text shadow first and then working on adding blur support as a separate patch.  If you'd like them all together, I can try to work on porting that code as part of this patch as well.

[Simon Hausmann]

Actually, a separate patch for the hasShadow() as well as the blur is fine with me. And this patch looks good to me :)

[Jonathon Jongsma (jonner)]

Created attachment 21883 [details]
minor cleanup to make the platform shadow functions empty

Here's a mini-patch that addresses the one other comment you had -- removing the functionality from the platform shadow functions.  For the other comments (hasShadow() and the blur implementations), I'll open new bugs.

[Simon Hausmann]

Comment on attachment 21843 [details]
Add text-shadow to the Qt port

Landed in r34763

[Simon Hausmann]

Comment on attachment 21883 [details]
minor cleanup to make the platform shadow functions empty

Looks good to me, landed in r34764 along with the removal of the TextShadow structure in GraphicsContextQt.

[Simon Hausmann]

Comment on attachment 21883 [details]
minor cleanup to make the platform shadow functions empty

Clearing r+ since the patch is in.

[Ariya Hidayat]

Comment on attachment 21843 [details]
Add text-shadow to the Qt port

BTW, any reason why the clip needs to be extended if there is shadow?

[Jonathon Jongsma (jonner)]

It's been so long since I did this that I don't remember exactly (in fact, I haven't worked on WebKit at all in quite some time).  My guess is that when I was implementing this, the text shadow fell outside of the clip area and so was not drawn unless I expanded the clip.  But as I said, I don't remember this patch very well.  Is it causing problems for you?