[Webkit-unassigned] [Bug 52535] offsetLeft is wrong for elements inside a TD whose style is set to position:relative

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #84273|review?                     |review-
               Flag|                            |




--- Comment #11 from Darin Adler <darin at apple.com>  2011-03-01 14:16:10 PST ---
(From update of attachment 84273)
View in context: https://bugs.webkit.org/attachment.cgi?id=84273&action=review

Great to tackle this. This fix is wrong, though.

> Source/WebCore/rendering/RenderObject.cpp:2161
> +      // If relative positioning is already set, then no need to look at the
> +      // raw style.

Should put that comment on one line.

> Source/WebCore/rendering/RenderObject.cpp:2169
> +      CSSStyleDeclaration* rawStyle = toElement(node())->style();
> +      RefPtr<CSSValue> position = rawStyle->getPropertyCSSValue(CSSPropertyPosition);
> +      if (position.get() && position->cssText() == "relative")
> +          return true;
> +      return false;

This works only if the style is set with a style attribute, not when it’s set with a stylesheet. You should add a test case where position:relative is set in the stylesheet, perhaps based on class or ID, and that test case will fail.

If we need this information we will probably have to put it in the RenderStyle. Going back to the DOM doesn’t seem workable because there’s no practical way to go back to all the various style declarations that could have influenced the style of the element.

The style declaration that comes from the style attribute for an element is not called the “raw style” and coining that term here is not a good idea. That one style declaration contains only a small part of what determines the style of an element.

Should not need the call to “.get()” here. And if (x) return true; return false; could just be replaced by return.

Getting the cssText is not the right way to check a CSSValue. The right way is more like this:

    static bool valueIsKeyword(CSSValue* value, int keyword)
    {
        return value && value->isPrimitiveValue() && static_cast<CSSPrimitiveValue*>(value)->getIdent() == keyword;
    }

    ...

    return valueIsKeyword(rawStyle->getPropertyCSSValue(CSSPropertyPosition).get(), CSSPropertyRelative);

But I think that’s beside the point since this is the wrong approach.

> Source/WebCore/rendering/RenderObject.h:405
> +    // WebKit doesn't support relative positioning for certain elements.
> +    // In such cases isRelPositioned() won't match the value set in the original style declaration.
> +    // This function returns the value in the raw style declaration.

I think this is a confusing comment. It doesn’t make it clear if this lack of support is a bug or a feature. Same comment as above about the term “raw style”.

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