Bug 59540

Summary: REGRESSION: white overlay scrollbars on apple.com/startpage
Product: WebKit Reporter: Jon Lee <jonlee>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, simon.fraser, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch
simon.fraser: review-, simon.fraser: commit-queue-
Patch none

Description Jon Lee 2011-04-26 14:47:29 PDT
<rdar://problem/9338653>
Comment 1 Jon Lee 2011-04-26 17:02:17 PDT
Created attachment 91185 [details]
Patch
Comment 2 WebKit Review Bot 2011-04-26 17:03:59 PDT
Attachment 91185 [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:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Jon Lee 2011-04-26 17:13:11 PDT
Created attachment 91191 [details]
Patch

fix ChangeLog
Comment 4 Simon Fraser (smfr) 2011-04-26 17:27:49 PDT
Comment on attachment 91191 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=91191&action=review

> Source/WebCore/page/Frame.cpp:510
> +    // <https://bugs.webkit.org/show_bug.cgi?id=59540> To find the aggregate background color
> +    // of the document, we look at the background color of the document and the body, and blend
> +    // them against the base background color of the frame view. Ideally we should include
> +    // background images, but including them in this calculation is time-intensive,
> +    // and would lead to unpredictable results.

I think this is too verbose (imagine if every change had such comments!).

> Source/WebCore/page/Frame.cpp:530
> +        return htmlBackgroundColor;

What if htmlBackgroundColor is transparent? Shouldn't this return the base background color?

> Source/WebCore/page/Frame.cpp:534
> +    if (!htmlBackgroundColor.isValid())
> +        return bodyBackgroundColor;

bodyBackgroundColor may be transparent here also, so you you'd need to return the baseBackgroundColor.

> Source/WebCore/page/Frame.cpp:543
> +    return view()->baseBackgroundColor().blend(htmlBackgroundColor.blend(bodyBackgroundColor));

Blend does linear interpolation, which is not the same as what is used for compositing. I think you want to combine them using the "normal" blend mode (aka Porter Duff's "source over"): R = S + D*(1 - Sa)
Comment 5 Jon Lee 2011-04-27 16:59:54 PDT
Created attachment 91386 [details]
Patch

Including the base background for short-circuit tests, and adding a comment in Color.h re: blend() function.
Comment 6 WebKit Commit Bot 2011-04-28 00:47:22 PDT
The commit-queue encountered the following flaky tests while processing attachment 91386 [details]:

http/tests/inspector/console-websocket-error.html bug 57392 (authors: pfeldman@chromium.org and yutak@chromium.org)
The commit-queue is continuing to process your patch.
Comment 7 WebKit Commit Bot 2011-04-28 00:48:56 PDT
Comment on attachment 91386 [details]
Patch

Clearing flags on attachment: 91386

Committed r85171: <http://trac.webkit.org/changeset/85171>
Comment 8 WebKit Commit Bot 2011-04-28 00:49:01 PDT
All reviewed patches have been landed.  Closing bug.