Bug 27841 – Fix box shadow painting: should pass shadow's color

• Component: CSS
• Status: RESOLVED INVALID

[Crystal Zhang]

In WebCore::RenderBoxModelObject::paintBoxShadow, should pass shadow's color when painting, not color::black.

[Crystal Zhang]

Created attachment 33791 [details]
patch to fix box shadow painting

[George Staikos]

You can remove the "Written by" part of the changelog and you should use your @torchmobile.com address probably.

Comment on attachment 33791 [details]
patch to fix box shadow painting

I think this is wrong. What is the issue this is supposed to address?

[Crystal Zhang]

(In reply to comment #3)
> (From update of attachment 33791 [details])
> I think this is wrong. What is the issue this is supposed to address?

It always uses Color::black to paint box shadow regardless shadow style, which is wrong.

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 33791 [details] [details])
> > I think this is wrong. What is the issue this is supposed to address?
> 
> It always uses Color::black to paint box shadow regardless shadow style, which
> is wrong.

The code uses a black fill for the shape casting the shadow. The shadow properties, including its color, are based on the ShadowData structure from the style:

    for (ShadowData* shadow = s->boxShadow(); shadow; shadow = shadow->next) {
[…]
        Color& shadowColor = shadow->color;
[…]
            context->setShadow(shadowOffset, shadowBlur, shadowColor);

[Crystal Zhang]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > (From update of attachment 33791 [details] [details] [details])
> > > I think this is wrong. What is the issue this is supposed to address?
> > 
> > It always uses Color::black to paint box shadow regardless shadow style, which
> > is wrong.
> 
> The code uses a black fill for the shape casting the shadow. The shadow
> properties, including its color, are based on the ShadowData structure from the
> style:
> 
>     for (ShadowData* shadow = s->boxShadow(); shadow; shadow = shadow->next) {
> […]
>         Color& shadowColor = shadow->color;
> […]
>             context->setShadow(shadowOffset, shadowBlur, shadowColor);

Thanks for your explain. Probably I misunderstood. One thing still confusing: can I know the reason that it uses shadow color later in that function?

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > > > (From update of attachment 33791 [details] [details] [details] [details])
> > > > I think this is wrong. What is the issue this is supposed to address?
> > > 
> > > It always uses Color::black to paint box shadow regardless shadow style, which
> > > is wrong.
> > 
> > The code uses a black fill for the shape casting the shadow. The shadow
> > properties, including its color, are based on the ShadowData structure from the
> > style:
> > 
> >     for (ShadowData* shadow = s->boxShadow(); shadow; shadow = shadow->next) {
> > […]
> >         Color& shadowColor = shadow->color;
> > […]
> >             context->setShadow(shadowOffset, shadowBlur, shadowColor);
> 
> Thanks for your explain. Probably I misunderstood. One thing still confusing:
> can I know the reason that it uses shadow color later in that function?

I am not sure which use of shadow color you are asking about. There are two branches, one for “normal” shadows and one for inset shadows, and each branch has its own call to setShadow() where it passes the shadow color.

[Crystal Zhang]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > (In reply to comment #4)
> > > > (In reply to comment #3)
> > > > > (From update of attachment 33791 [details] [details] [details] [details] [details])
> > > > > I think this is wrong. What is the issue this is supposed to address?
> > > > 
> > > > It always uses Color::black to paint box shadow regardless shadow style, which
> > > > is wrong.
> > > 
> > > The code uses a black fill for the shape casting the shadow. The shadow
> > > properties, including its color, are based on the ShadowData structure from the
> > > style:
> > > 
> > >     for (ShadowData* shadow = s->boxShadow(); shadow; shadow = shadow->next) {
> > > […]
> > >         Color& shadowColor = shadow->color;
> > > […]
> > >             context->setShadow(shadowOffset, shadowBlur, shadowColor);
> > 
> > Thanks for your explain. Probably I misunderstood. One thing still confusing:
> > can I know the reason that it uses shadow color later in that function?
> 
> I am not sure which use of shadow color you are asking about. There are two
> branches, one for “normal” shadows and one for inset shadows, and each branch
> has its own call to setShadow() where it passes the shadow color.

The branch for inset shadows, it calls fillRoundedRect and fillRect with shadow color.

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > > > (In reply to comment #4)
> > > > > (In reply to comment #3)
> > > > > > (From update of attachment 33791 [details] [details] [details] [details] [details] [details])
> > > > > > I think this is wrong. What is the issue this is supposed to address?
> > > > > 
> > > > > It always uses Color::black to paint box shadow regardless shadow style, which
> > > > > is wrong.
> > > > 
> > > > The code uses a black fill for the shape casting the shadow. The shadow
> > > > properties, including its color, are based on the ShadowData structure from the
> > > > style:
> > > > 
> > > >     for (ShadowData* shadow = s->boxShadow(); shadow; shadow = shadow->next) {
> > > > […]
> > > >         Color& shadowColor = shadow->color;
> > > > […]
> > > >             context->setShadow(shadowOffset, shadowBlur, shadowColor);
> > > 
> > > Thanks for your explain. Probably I misunderstood. One thing still confusing:
> > > can I know the reason that it uses shadow color later in that function?
> > 
> > I am not sure which use of shadow color you are asking about. There are two
> > branches, one for “normal” shadows and one for inset shadows, and each branch
> > has its own call to setShadow() where it passes the shadow color.
> 
> The branch for inset shadows, it calls fillRoundedRect and fillRect with shadow
> color.

If the hole rect is empty then the shadow is cast uniformly on the entire content box. In the case, instead of using a separate shape to cast the shadow, the code just applies a uniform fill with the shadow color.

[Crystal Zhang]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #6)
> > > > (In reply to comment #5)
> > > > > (In reply to comment #4)
> > > > > > (In reply to comment #3)
> > > > > > > (From update of attachment 33791 [details] [details] [details] [details] [details] [details] [details])
> > > > > > > I think this is wrong. What is the issue this is supposed to address?
> > > > > > 
> > > > > > It always uses Color::black to paint box shadow regardless shadow style, which
> > > > > > is wrong.
> > > > > 
> > > > > The code uses a black fill for the shape casting the shadow. The shadow
> > > > > properties, including its color, are based on the ShadowData structure from the
> > > > > style:
> > > > > 
> > > > >     for (ShadowData* shadow = s->boxShadow(); shadow; shadow = shadow->next) {
> > > > > […]
> > > > >         Color& shadowColor = shadow->color;
> > > > > […]
> > > > >             context->setShadow(shadowOffset, shadowBlur, shadowColor);
> > > > 
> > > > Thanks for your explain. Probably I misunderstood. One thing still confusing:
> > > > can I know the reason that it uses shadow color later in that function?
> > > 
> > > I am not sure which use of shadow color you are asking about. There are two
> > > branches, one for “normal” shadows and one for inset shadows, and each branch
> > > has its own call to setShadow() where it passes the shadow color.
> > 
> > The branch for inset shadows, it calls fillRoundedRect and fillRect with shadow
> > color.
> 
> If the hole rect is empty then the shadow is cast uniformly on the entire
> content box. In the case, instead of using a separate shape to cast the shadow,
> the code just applies a uniform fill with the shadow color.

Got it. Thank you very much!