Bug 129004

Summary: UIProcess needs to know the color of the page's extended background
Product: WebKit Reporter: Beth Dakin <bdakin>
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, commit-queue, esprehn+autocc, glenn, kondapallykalyan, simon.fraser, thorton
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch bfulgham: review+

Description Beth Dakin 2014-02-18 14:50:07 PST
The UIProcess needs to know the color of the page's extended background.
Comment 1 Beth Dakin 2014-02-18 15:12:14 PST
Created attachment 224557 [details]
Patch
Comment 2 WebKit Commit Bot 2014-02-18 15:14:46 PST
Attachment 224557 [details] did not pass style-queue:


ERROR: Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:872:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Beth Dakin 2014-02-18 15:47:47 PST
Created attachment 224561 [details]
Patch

Make style bot happy.
Comment 4 Brent Fulgham 2014-02-19 11:24:57 PST
Comment on attachment 224561 [details]
Patch

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

r=me

> Source/WebKit2/ChangeLog:8
> +        New method on WKWebView and WKView will return the pageâs extended background 

Yuck. Looks like a non-ASCII apostrophe.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:531
> +    return [UIColor colorWithRed:(color.red() / 255.0) green:(color.green() / 255.0) blue:(color.blue() / 255.0) alpha:(color.alpha() / 255.0)];

It's too bad we don't have a helper function for this, so that WebCore::Color->UIColor is always done the same way.
Comment 5 Tim Horton 2014-02-19 11:29:32 PST
Comment on attachment 224561 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:531
>> +    return [UIColor colorWithRed:(color.red() / 255.0) green:(color.green() / 255.0) blue:(color.blue() / 255.0) alpha:(color.alpha() / 255.0)];
> 
> It's too bad we don't have a helper function for this, so that WebCore::Color->UIColor is always done the same way.

We really ought to, similar to the nsColor() one in ColorMac.
Comment 6 Simon Fraser (smfr) 2014-02-19 11:38:16 PST
Comment on attachment 224561 [details]
Patch

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

>>> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:531
>>> +    return [UIColor colorWithRed:(color.red() / 255.0) green:(color.green() / 255.0) blue:(color.blue() / 255.0) alpha:(color.alpha() / 255.0)];
>> 
>> It's too bad we don't have a helper function for this, so that WebCore::Color->UIColor is always done the same way.
> 
> We really ought to, similar to the nsColor() one in ColorMac.

When this color has alpha, what gets shown underneath?
Comment 7 Simon Fraser (smfr) 2014-02-19 12:36:40 PST
r=me for WK2
Comment 8 Andreas Kling 2014-02-19 12:38:44 PST
Comment on attachment 224561 [details]
Patch

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

> Source/WebCore/rendering/RenderLayerCompositor.cpp:263
> +    , m_rootExtendedBackgroundColor(Color())

This line is not needed.
Comment 9 Beth Dakin 2014-02-19 12:43:05 PST
Simon and I talked about the alpha stuff in person. http://trac.webkit.org/changeset/164382