Bug 25343 - -webkit-box-shadow with 0,0 offset ignores blur
Summary: -webkit-box-shadow with 0,0 offset ignores blur
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-23 07:50 PDT by Nate Chapin
Modified: 2009-04-24 11:44 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.04 KB, patch)
2009-04-23 07:54 PDT, Nate Chapin
dglazkov: review-
Details | Formatted Diff | Diff
Patch2 (1.30 KB, patch)
2009-04-24 10:19 PDT, Nate Chapin
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Chapin 2009-04-23 07:50:48 PDT
Regression introduced in http://trac.webkit.org/changeset/42375

If -webkit-box-shadow is used with height and width of 0, the blur value is being ignored.  This is causing LayoutTests/fast/repaint/box-shadow-h.html and LayoutTests/fast/repaint/box-shadow-v.html to fail.
Comment 1 Nate Chapin 2009-04-23 07:54:30 PDT
Created attachment 29711 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2009-04-24 10:05:23 PDT
Comment on attachment 29711 [details]
Patch

Good work and welcome :)

A couple of things about ChangeLog contents. It's actually very handy (like when perusing trac), so we really try to put more details there. This would be a good ChangeLog entry:

2009-04-22  Nate Chapin  <japhet@google.com>

        Reviewed by NOBODY (OOPS!).

        https://bugs.webkit.org/show_bug.cgi?id=25343
        Fix Chromium/Skia bug where -webkit-box-shadow with 0,0 offset ignores blur.

        * WebCore\platform\graphics\skia\GraphicsContextSkia.cpp:
        (WebCore::GraphicsContext::setPlatformShadow): Added an extra check for blur.

In other words:

 * URL of the bug
 * Brief description of the bug
 * For each change, brief documentation of what it is.

r-, but only because of this.

> +2009-04-22  Nate Chapin  <japhet@google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * WebCore\platform\graphics\skia\GraphicsContextSkia.cpp
> +:
> +

> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 42752)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,10 @@
> +2009-04-22  Nate Chapin  <japhet@google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * WebCore\platform\graphics\skia\GraphicsContextSkia.cpp
> +:
> +
>  2009-04-14  Xan Lopez  <xlopez@igalia.com>
>  
>          Unreviewed version bump in preparation for 1.1.5 release.
> Index: WebCore/platform/graphics/skia/GraphicsContextSkia.cpp
> ===================================================================
> --- WebCore/platform/graphics/skia/GraphicsContextSkia.cpp	(revision 42752)
> +++ WebCore/platform/graphics/skia/GraphicsContextSkia.cpp	(working copy)
> @@ -963,7 +963,7 @@ void GraphicsContext::setPlatformShadow(
>          return;
>  
>      // Detect when there's no effective shadow and clear the looper.
> -    if (size.width() == 0 && size.height() == 0) {
> +    if (size.width() == 0 && size.height() == 0 && blurInt == 0) {
>          platformContext()->setDrawLooper(NULL);
>          return;
>      }
Comment 3 Nate Chapin 2009-04-24 10:19:05 PDT
Created attachment 29747 [details]
Patch2
Comment 4 Dimitri Glazkov (Google) 2009-04-24 10:23:25 PDT
Comment on attachment 29747 [details]
Patch2

There's an errant floatey colon at the end of ChangeLog, but I'll clean up when landing.
Comment 5 Dimitri Glazkov (Google) 2009-04-24 11:44:05 PDT
Landed as http://trac.webkit.org/changeset/42833.