Bug 36793

Summary:

CSS outline property on a narrow element has rabbit ear artifacts

Product:

WebKit

Reporter:

James Robinson <jamesr>

Component:

CSS

Assignee:

Robert Hogan <robert>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

robert, simon.fraser, tonikitoo

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Test cases	none
Patch	none
patch	none
Patch	none
Patch	simon.fraser: review+, simon.fraser: commit-queue-

James Robinson

Reported 2010-03-29 16:59:18 PDT

With the following HTML: <!DOCTYPE html> the outline around the span has a non-rectangular top and bottom. It sort of looks like we're not generating enough strokes for very narrow outlines.

Attachments
Test cases (459 bytes, text/html) 2010-03-29 17:03 PDT, James Robinson	no flags	Details
Patch (5.51 KB, patch) 2011-06-15 20:05 PDT, Jason Liu	no flags	Details Formatted Diff Diff
patch (3.86 KB, patch) 2011-06-17 03:24 PDT, Jason Liu	no flags	Details Formatted Diff Diff
Patch (5.10 KB, patch) 2011-06-19 18:46 PDT, Jason Liu	no flags	Details Formatted Diff Diff
Patch (7.43 KB, patch) 2011-09-21 14:55 PDT, Robert Hogan	simon.fraser: review+ simon.fraser: commit-queue-	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

James Robinson

Comment 1 2010-03-29 17:03:31 PDT

Created attachment 51984 [details] Test cases We do not match Firefox 3.6's rendering exactly on any of these four cases.

Jason Liu

Comment 2 2011-06-15 20:05:27 PDT

Created attachment 97392 [details] Patch Patch

Simon Fraser (smfr)

Comment 3 2011-06-15 20:09:58 PDT

Comment on attachment 97392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97392&action=review r- for the test > LayoutTests/ChangeLog:9 > + * fonts/outline-expected.txt: Added. > + * fonts/outline.html: Added. Why is the test in fonts? I'd suggest fast/css I think the test can also be dumpAsText(true) since you only care about the pixel result. The test should contain no text, and use large outlines to make the fix very obvious. You also need to include a pixel result in your patch.

Jason Liu

Comment 4 2011-06-17 03:24:30 PDT

Created attachment 97571 [details] patch I've changed the test case.

Simon Fraser (smfr)

Comment 5 2011-06-17 07:40:00 PDT

Comment on attachment 97571 [details] patch Can you include binary fixes in your patch (or use webkit-patch upload)?

Jason Liu

Comment 6 2011-06-19 18:46:33 PDT

Created attachment 97730 [details] Patch This is webkit-patch.

Robert Hogan

Comment 7 2011-08-10 13:26:37 PDT

Comment on attachment 97730 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=97730&action=review > LayoutTests/fast/css/outline-narrowLine.html:1 > +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/TR/REC-html40/loose.dtd"> Is this important to the test? The DOCTYPE used elsewhere, when specified, seems to be: <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> But if it doesn't affect the behaviour of the test I would suggest leave it out. > Source/WebCore/rendering/RenderInline.cpp:1527 > + > + if (thisline.x() == thisline.maxX()) > + drawLineForBoxSide(graphicsContext, > + l - ow, > + b, > + r + ow, > + b + ow, > + BSBottom, outlineColor, os, > + ow, > + ow, > + antialias > + ); > + This needs rebasing - the function's variable names have changed.

Robert Hogan

Comment 8 2011-09-21 14:55:45 PDT

Created attachment 108241 [details] Patch

Robert Hogan

Comment 9 2011-09-21 14:59:08 PDT

Note that for: WebKit, unlike firefox, does nothing. This is because empty inlines do not get a renderer in WebKit.

Simon Fraser (smfr)

Comment 10 2011-09-21 16:06:29 PDT

Comment on attachment 108241 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108241&action=review > LayoutTests/fast/css/outline-narrowLine.html:22 > +<html> > +<body> > + > + > + > + > + > + > + > + > + > + > + > + > + x > + > + > + > + > +x > +</body> > +</html> Can you make this test accentuate the outline? Try 50px-wide outlines.

Robert Hogan

Comment 11 2011-09-22 09:13:59 PDT

Committed r95721: <http://trac.webkit.org/changeset/95721>

Note You need to log in before you can comment on or make changes to this bug.