Bug 44719 – Fix RenderStyle::addCursor to use a StyleImage, not a CachedImage

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Simon Fraser (smfr)]

Per comment in CSSStyleSelector:
                     // FIXME: Temporary clumsiness to pass off a CachedImage to an API that will eventually convert to using
                    // StyleImage.

[Simon Fraser (smfr)]

Created attachment 65786 [details]
Patch

[Darin Adler]

Comment on attachment 65786 [details]
Patch

>          case CSSPropertyBackgroundImage:
> +            // FIXME: broken for multiple backgrounds.

It would be better if this was capitalized sentence-style like our comments usually are, and if there was a corresponding bug umber.

> +            case CSSPropertyCursor: {
> +                if (CursorList* cursorList = m_style->cursors()) {
> +                    for (size_t i = 0; i < cursorList->size(); ++i) {
> +                        CursorData& currentCursor = (*cursorList)[i];
> +                        if (currentCursor.image()->isPendingImage()) {
> +                            CSSImageValue* imageValue = static_cast<StylePendingImage*>(currentCursor.image())->cssImageValue();
> +                            currentCursor.setImage(imageValue->cachedImage(docLoader));
> +                        }
> +                    }
> +                }
>                  break;
> +            }

Is it correct to do nothing if there are not cursors?

> +    void setImage(StyleImage* image) { m_image = image; }    

This function should take a PassRefPtr, not a raw pointer.

> +    void addCursor(StyleImage*, const IntPoint& = IntPoint());

Should this take a PassRefPtr instead of a raw pointer? The IntPoint argument should have a name, hotSpot, instead of being unnamed. I don't think it's obvious what it is without a name.

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/66391