Bug 126124 – [cairo] Entering text into forms on github.com creates a trapezoid artifact

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Michael Catanzaro]

When typing in text boxes on github.com, sometimes a blue trapezoid artifact is created in the background of the text field.  This does not seem to occur in other browsers.

Unfortunately, the trapezoid disappears whenever I attempt to take a screenshot, and I'm not entirely sure what triggers it. However, a reliable reproducer -- if you have a GitHub account -- is to find some text field, hit the Enter key once or twice, then Ctrl+A to select all. The blue trapezoid appears underneath the selection, and remains if you hit the down arrow key to deselect everything.

webkitgtk3-2.2.3-1.fc20
epiphany-3.10.2-1.fc20

[Michael Catanzaro]

Created attachment 219855 [details]
screenshot of artifact

Just kidding: the blue trapezoid appeared to disappear immediately before the screenshots were taken, but it actually does appear in the screenshots. Here's one.

[Michael Catanzaro]

Can also be gray (GNOME #759376).

[Jérémy Lal]

Created attachment 290312 [details]
another artifact with a triangular shape

[Cédric Bellegarde]

Can confirm this issue but I think composent is wrong as it doesn't happen on Safari. Should be moved to GTK.

[Gwang Yoon Hwang]

Created attachment 306662 [details]
If you see the red when you input the space, it faiils.

This bug happens when we are drawing the border of the inline box which has a
complex border (such as dashed, dotted..) with a border radius property.

When we made a change inside of text box, for example, typing space, we are 
going to invalidate the area of the inline box.

While rendering decorations of the render box,
RenderBoxModelObject::paintBorder defines a clip out region to prepare the inner
border to avoid the background bleeding out behind the border.

We are facing a problem from now on. 

The RenderBoxModelObject is going to make a clip to render each side of the
border area. But when we apply this clipping to the cairo context, it seems
cairo context looses the previous clipped out area, which was defined to avoid
bleeding.

It is okay when we draw dashed border side since it is a stroke. But when we
draw a solid border side, it uses fill operation which would fill the whole
quad of the border side with a border color.

I tried to make simple cairo problem to reproduce the problem but it is not so
successful so far.

[Gwang Yoon Hwang]

It is a bug of Cairo while handling clipping regions.

I made unit test cases for Cairo and made a bug:
https://bugs.freedesktop.org/show_bug.cgi?id=100722

I also made a small optimization patch which avoids the this condition,
https://bugs.webkit.org/show_bug.cgi?id=170988

But it just removes this problem when there is no scrolling area.

[Michael Catanzaro]

OK, thanks Yoon! It looks like there are no further changes to be made in WebKit, so closing this now.

[Sergio Villar Senin]

Why not keeping this open till the cairo bug is not fixed? I find it interesting for tracking purposes. We could use this very same bug later when updating the dependencies with the cairo version.

[Michael Catanzaro]

Please no, only keep WebKit bugs open if a source code change to WebKit is required. The GitHub issue is fixed anyway.

[Sergio Villar Senin]

(In reply to Michael Catanzaro from comment #9)
> Please no, only keep WebKit bugs open if a source code change to WebKit is
> required. The GitHub issue is fixed anyway.

That might be you, that is not a general policy and different people do different things.

Secondly, I don't know what you mean with fixed, but the Cairo patch is still waiting for review so still far from landing and being added to a release we could depend on.

[Michael Catanzaro]

> Secondly, I don't know what you mean with fixed, but the Cairo patch is
> still waiting for review so still far from landing and being added to a
> release we could depend on.

Yoon committed a patch to work around the problem last week.

[Cédric Bellegarde]

Just upgraded to webkit2gtk 2.16.2 and issue is always present.

Happen less I think but happen everytime I post a comment on github.

[Adrian Perez]

(In reply to Cédric Bellegarde from comment #12)
> Just upgraded to webkit2gtk 2.16.2 and issue is always present.
> 
> Happen less I think but happen everytime I post a comment on github.

The shortlist of patches merged into the 2.16.2 release is here:

  https://trac.webkit.org/wiki/WebKitGTK/2.16.x

It does not seem to include the fix for this bug.

[Adrian Perez]

(CC'ing Carlos García, who usually makes our releases.)

Carlos, do you think it would be feasible to include this fix in
an upcoming 2.16.x release?

[Gwang Yoon Hwang]

(In reply to Adrian Perez from comment #14)
> (CC'ing Carlos García, who usually makes our releases.)
> 
> Carlos, do you think it would be feasible to include this fix in
> an upcoming 2.16.x release?

(In reply to Cédric Bellegarde from comment #12)
> Just upgraded to webkit2gtk 2.16.2 and issue is always present.
> 
> Happen less I think but happen everytime I post a comment on github.

I patched to skip border paintings in certain conditions and that patch reduces artifacts a bit.
But to fix this problem, we need to fix a clipping bug in Cairo or we need to change a way to render box decorations, which is quite different scope of work.

This bug does not include a "fix" for this bug.

[Carlos Garcia Campos]

(In reply to Adrian Perez from comment #13)
> (In reply to Cédric Bellegarde from comment #12)
> > Just upgraded to webkit2gtk 2.16.2 and issue is always present.
> > 
> > Happen less I think but happen everytime I post a comment on github.
> 
> The shortlist of patches merged into the 2.16.2 release is here:
> 
>   https://trac.webkit.org/wiki/WebKitGTK/2.16.x

No no no. This is not the list of patches merges, this is the list of patches people request me to merge. You should add commits there once they land in trunk and you think they should be backported to stable branch. The list of commits actually merged is this:

http://trac.webkit.org/log/webkit/releases/WebKitGTK/webkit-2.16

> It does not seem to include the fix for this bug.

And it does include the workaround for this:

http://trac.webkit.org/changeset/216373/webkit/releases/WebKitGTK/webkit-2.16

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #11)
> Yoon committed a patch to work around the problem last week.

Anyway it clearly doesn't work; the trapezoid appears just like it did five years ago.

The bug has migrated: https://gitlab.freedesktop.org/cairo/cairo/issues/297

I wonder if this affects WinCairo?

[Fujii Hironori]

Created attachment 356938 [details]
Screenshot of WinCairo WebKitLegacy

(In reply to Michael Catanzaro from comment #17)
> I wonder if this affects WinCairo?

It's relatively easy to reproduce within WinCairo WK1 window with Comment 5 test case and GitHub textarea.
It's much difficult to reproduce within WinCairo WK2 window, but I'm able to reproduce it with GitHub textarea.

[Carlos Garcia Campos]

I'm not so sure this is a cairo bug, even though it could be fixed in cairo. I think it's actually a limitation, since the case is handled in the code. See:

https://gitlab.freedesktop.org/cairo/cairo/blob/master/src/cairo-clip-polygon.c#L49

_cairo_clip_get_polygon() calls can_convert_to_polygon() and returns early with CAIRO_INT_STATUS_UNSUPPORTED if it fails. can_convert_to_polygon() simply checks that all paths in the chain have the same antialiasing. So, mixing antialiasing modes in the same clip is not actually supported by cairo. In the case of rectangle clips we are already ignoring the current antialiasing to not do any antialiasing. We could do the opposite for clips receiving a path, we want to enforce antialiasing in that case since the path might contain curves. Doing that we ensure all calls to clip with a path use the same antialiasing, which is the case of the github bug.

[Carlos Garcia Campos]

Created attachment 372047 [details]
Patch

[Michael Catanzaro]

Comment on attachment 372047 [details]
Patch

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

Thanks so much for fixing this!

You forgot the ChangeLog entry.

> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:1276
> -    cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr);
> -    cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
> -    cairo_clip(cr);
> +//    cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr);
> +//    cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
> +//    cairo_clip(cr);
> +    doClipWithAntialias(cr, CAIRO_ANTIALIAS_NONE);

You also forgot to remove the original code. Try again. :)

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #21)
> Comment on attachment 372047 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=372047&action=review
> 
> Thanks so much for fixing this!
> 
> You forgot the ChangeLog entry.
> 
> > Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:1276
> > -    cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr);
> > -    cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
> > -    cairo_clip(cr);
> > +//    cairo_antialias_t savedAntialiasRule = cairo_get_antialias(cr);
> > +//    cairo_set_antialias(cr, CAIRO_ANTIALIAS_NONE);
> > +//    cairo_clip(cr);
> > +    doClipWithAntialias(cr, CAIRO_ANTIALIAS_NONE);
> 
> You also forgot to remove the original code. Try again. :)

Oops, I actually submitted the wrong diff, let me upload the right one.

[Carlos Garcia Campos]

Created attachment 372057 [details]
Patch

[Carlos Garcia Campos]

Committed r246431: <https://trac.webkit.org/changeset/246431>

[Radar WebKit Bug Importer]

<rdar://problem/51739186>