Bug 60157

Summary:

Gradients not horizontal using 270deg with odd div width

Product:

WebKit

Reporter:

Martin Wittemann <martin.wittemann>

Component:

CSS

Assignee:

Mihnea Ovidenie <mihnea>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, shanestephens, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

OS X 10.6

Attachments:

Description	Flags
Test-Case	none
Patch	simon.fraser: review-
Patch2	simon.fraser: review-
Patch3	none

Martin Wittemann

Reported 2011-05-04 04:17:34 PDT

Assigning a gradient like this background-image: -webkit-linear-gradient(270deg, #CCCCCC 50%, #999999 50%); leads to a strange result in cases the width of the div using that style rule is odd. Giving the same div an even width, everything looks like it should. Just check out the attached HTML file to see the difference. I have seen it in chrome on OSX and Linux so I guess its not OS dependent.

Attachments
Test-Case (349 bytes, text/html) 2011-05-04 11:43 PDT, Martin Wittemann	no flags	Details
Patch (76.11 KB, patch) 2011-05-13 09:13 PDT, Mihnea Ovidenie	simon.fraser: review-	Details Formatted Diff Diff
Patch2 (74.54 KB, patch) 2011-05-16 01:24 PDT, Mihnea Ovidenie	simon.fraser: review-	Details Formatted Diff Diff
Patch3 (105.33 KB, patch) 2011-05-16 23:55 PDT, Mihnea Ovidenie	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Simon Fraser (smfr)

Comment 1 2011-05-04 11:10:05 PDT

Did you forget to attach the file?

Martin Wittemann

Comment 2 2011-05-04 11:43:03 PDT

Created attachment 92291 [details] Test-Case Seems like I forgot it. Sorry, here it is.

Mihnea Ovidenie

Comment 3 2011-05-13 09:13:18 PDT

Created attachment 93460 [details] Patch The code in CSSGradientValue.cpp, function endPointsFromAngle, was missing the case when the angle is 270. The patch adds the computation for this case too. The layout test creates linear gradients with angles = {0, 90, 180, 270, -90, -180, -270}.

Simon Fraser (smfr)

Comment 4 2011-05-13 09:25:12 PDT

Comment on attachment 93460 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93460&action=review The code change looks good but I'd like to see the test tweaked. > LayoutTests/fast/gradients/css3-linear-right-angle-gradients.html:6 > +<title> > + Test that computation of right angle (90, 180, 270) linear gradients is done properly. > +</title> This should be a dumpAsText(true) test. Also, it would be better if the divs were squares (inline-block to make them line up), and don't use red if red is expected in the test result.

Mihnea Ovidenie

Comment 5 2011-05-16 01:24:50 PDT

Created attachment 93621 [details] Patch2 Thanks for the review. 1. I have added the dumpAsText, i missed it completely in the first place. 2. I decided to keep the divs in the original, long shape instead of making them squares. This way, the problem with the gradient is highlighted better. 3. I changed the colors from green-red to write-black, thanks for the suggestion. Regards,

Simon Fraser (smfr)

Comment 6 2011-05-16 07:59:16 PDT

Comment on attachment 93621 [details] Patch2 View in context: https://bugs.webkit.org/attachment.cgi?id=93621&action=review > LayoutTests/fast/gradients/css3-linear-right-angle-gradients.html:6 > +<!DOCTYPE html> > +<html> > +<head> > +<title> > + Test that computation of right angle (90, 180, 270) linear gradients is done properly. > +</title> Please make the output look similar to that of css3-repeating-radial-gradients.html, with no text, and larger boxes.

Mihnea Ovidenie

Comment 7 2011-05-16 23:55:41 PDT

Created attachment 93742 [details] Patch3 The third version of the patch. I have reworked the layout test.

WebKit Commit Bot

Comment 8 2011-05-19 02:19:19 PDT

Comment on attachment 93742 [details] Patch3 Clearing flags on attachment: 93742 Committed r86826: <http://trac.webkit.org/changeset/86826>

WebKit Commit Bot

Comment 9 2011-05-19 02:19:25 PDT

All reviewed patches have been landed. Closing bug.

Martin Wittemann

Comment 10 2011-05-23 05:23:04 PDT

Thanks for fixing it. Just checked it with the current webkit and it works. (Don't know if I'm allowed to set the status to verified so I better keep it the way it is)

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