RESOLVED FIXED 36793
CSS outline property on a narrow element has rabbit ear artifacts 
https://bugs.webkit.org/show_bug.cgi?id=36793
Summary CSS outline property on a narrow element has rabbit ear artifacts
James Robinson
Reported 2010-03-29 16:59:18 PDT
With the following HTML: <!DOCTYPE html> <span style="outline: solid red 5px"><br><span> 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
DetailsFormatted DiffDiff
patch (3.86 KB, patch)
2011-06-17 03:24 PDT, Jason Liu 		no flags
DetailsFormatted DiffDiff
Patch (5.10 KB, patch)
2011-06-19 18:46 PDT, Jason Liu 		no flags
DetailsFormatted DiffDiff
Patch (7.43 KB, patch)
2011-09-21 14:55 PDT, Robert Hogan 		simon.fraser: review+
simon.fraser: commit-queue-
DetailsFormatted DiffDiff
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: <span style="outline: solid red 5px"></span> 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> > +<!--Outline around an empty span: WebKit does nothing here, as empty inlines are ignored.--> > +<p> > +<span style="outline: solid red 5px"></span> > +<p> > + > +<!--Outline around a span with a line break:--> > +<p> > +<span style="outline: solid green 5px"><br></span> > +<p> > + > +<!--Outline around a span with a line break and an 'x':--> > +<p> > +<span style="outline: solid blue 5px"><br>x</span> > +<p> > + > +<!--Outline around a span with an 'x' and two line breaks:--> > +<p> > +<span style="outline: solid purple 5px">x<br><br></span> > +</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.