Bug 207758

Created attachment 390758 [details]
Patch

Comment on attachment 390758 [details]
Patch

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

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:298
> +    UNUSED_PARAM(offset);

What is the reason for these two being ignored? IIUC, it is possible
to set some CSS properties on focus rings using the :focus selector,
and I think these two values might comes from there in some way e.g.
if the shape is not overriden, but only the border width then that
would mean “paint the system focus ring, but with a custom width”.

I think it's fine to land the patch as-is, but I would leave a TODO
comment here.

> Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:320
> +    UNUSED_PARAM(offset);

Ditto.

(In reply to Adrian Perez from comment #2)
> Comment on attachment 390758 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=390758&action=review
> 
> > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:298
> > +    UNUSED_PARAM(offset);
> 
> What is the reason for these two being ignored?

We have always ignored them in GTK port. I checked other browsers and they also seem to ignore it.

> IIUC, it is possible
> to set some CSS properties on focus rings using the :focus selector,
> and I think these two values might comes from there in some way e.g.
> if the shape is not overriden, but only the border width then that
> would mean “paint the system focus ring, but with a custom width”.

I tried to honor them and it didn't really work. In the case of the offset, it sometimes applied already by renderer (depends on the caller).

> I think it's fine to land the patch as-is, but I would leave a TODO
> comment here.

Ok, we can investigate it later.

> > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:320
> > +    UNUSED_PARAM(offset);
> 
> Ditto.

Committed r257049: <https://trac.webkit.org/changeset/257049>