[Webkit-unassigned] [Bug 27573] [CSS3 Backgrounds and Borders] Add support for the "contain" value for background-size

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27573





--- Comment #2 from mitz at webkit.org  2009-08-19 14:18:38 PDT ---
(From update of attachment 35132)
Review comments. I has them.

> +        Return contain or cover when appropriate, and otherwise so what we 
> +        used to do.

Typo: “so”.

> +        backgroundSize() now returns an EBackgroundSize, and 
> +        backgroundSizeLength() returns the numbers

I think you meant “lengths” since they can still be 'auto' or a percentage.



>  #
> +# css_prop_background_size
> +#

We no longer have these weird enum values for the properties, and I never
understood why the comments were in term of those. You can just say
“background-size”.

> Index: WebCore/rendering/RenderBoxModelObject.cpp
> ===================================================================
> --- WebCore/rendering/RenderBoxModelObject.cpp	(revision 47461)
> +++ WebCore/rendering/RenderBoxModelObject.cpp	(working copy)
> @@ -497,8 +497,8 @@ IntSize RenderBoxModelObject::calculateB
>      if (bgLayer->isSizeSet()) {

Not sure why you didn’t change this one to isSizeLengthSet()…

> Index: WebCore/rendering/RenderObject.cpp
> ===================================================================
> --- WebCore/rendering/RenderObject.cpp	(revision 47461)
> +++ WebCore/rendering/RenderObject.cpp	(working copy)
> @@ -656,8 +656,8 @@ static bool mustRepaintFillLayers(const 
>      if (!layer->xPosition().isZero() || !layer->yPosition().isZero())
>          return true;
>  
> -    if (layer->isSizeSet()) {
> -        if (layer->size().width().isPercent() || layer->size().height().isPercent())
> +    if (layer->isSizeLengthSet()) {
> +        if (layer->sizeLength().width().isPercent() || layer->sizeLength().height().isPercent())

…because you did change this one.

I think instead of adding another “is set” bit, you can get rid of the existing
one and just add a value to the EBackgroundSize enum representing the “not set”
state. But now I am not sure I fully understand why we need this notion in the
first place and what’s the difference between “not set” and 'background-size:
auto, auto'.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.