Bug 59540 - REGRESSION: white overlay scrollbars on apple.com/startpage
Summary: REGRESSION: white overlay scrollbars on apple.com/startpage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-04-26 14:47 PDT by Jon Lee
Modified: 2011-04-28 00:49 PDT (History)
3 users (show)

See Also:


Attachments
Patch (4.56 KB, patch)
2011-04-26 17:02 PDT, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (4.66 KB, patch)
2011-04-26 17:13 PDT, Jon Lee
simon.fraser: review-
simon.fraser: commit-queue-
Details | Formatted Diff | Diff
Patch (5.43 KB, patch)
2011-04-27 16:59 PDT, Jon Lee
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.