Bug 132534 - [Extended Background] Respect repeat-x/repeat-y when creating margin tiles
Summary: [Extended Background] Respect repeat-x/repeat-y when creating margin tiles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-03 15:27 PDT by Sam Weinig
Modified: 2014-05-06 09:45 PDT (History)
6 users (show)

See Also:


Attachments
Patch (14.21 KB, patch)
2014-05-03 15:29 PDT, Sam Weinig
bdakin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2014-05-03 15:27:05 PDT
[Extended Background] Respect repeat-x/repeat-y when creating margin tiles
Comment 1 Sam Weinig 2014-05-03 15:29:27 PDT
Created attachment 230767 [details]
Patch
Comment 2 Simon Fraser (smfr) 2014-05-03 15:41:51 PDT
Comment on attachment 230767 [details]
Patch

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

> Source/WebCore/page/FrameView.cpp:2547
> +    unsigned mode = 0;

Can't this be a type?

> Source/WebCore/page/FrameView.cpp:2561
> +    if (documentElementRenderer && documentElementRenderer->style().hasBackgroundImage()) {
> +        if (documentElementRenderer->style().backgroundRepeatX() == RepeatFill)
> +            mode |= ExtendedBackgroundModeHorizontal;
> +        if (documentElementRenderer->style().backgroundRepeatY() == RepeatFill)
> +            mode |= ExtendedBackgroundModeVertical;
> +    }
> +    
> +    if (bodyRenderer && bodyRenderer->style().hasBackgroundImage()) {
> +        if (bodyRenderer->style().backgroundRepeatX() == RepeatFill)
> +            mode |= ExtendedBackgroundModeHorizontal;
> +        if (bodyRenderer->style().backgroundRepeatY() == RepeatFill)
> +            mode |= ExtendedBackgroundModeVertical;
> +    }

I think you only want to look at the one that supplies the root background. We have (poorly factored) code elsewhere that does this.

> Source/WebCore/page/FrameView.cpp:2592
> +    renderView->compositor().setRootExtendedBackgroundColor(mode == ExtendedBackgroundModeAll ? Color() : documentBackgroundColor());

Any reason not to always set the color? What happens if the background image has alpha?

> Source/WebCore/page/FrameView.cpp:2595
> +    

Boop.

> Source/WebCore/page/FrameView.cpp:2614
> +    

Stupid xcode.

> Source/WebCore/page/FrameView.h:203
> +    };
> +    

Typedef for the bitmask please.

> Source/WebCore/page/FrameView.h:210
> +    

r-
Comment 3 Beth Dakin 2014-05-05 13:42:23 PDT
Comment on attachment 230767 [details]
Patch

This is a good approach. r=me, but you should address Simon's comments.
Comment 4 Sam Weinig 2014-05-06 09:45:28 PDT
Committed revision 168362.