WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
14442
Adding a stop with value 1.0 to a gradient that has already been used has no effect
https://bugs.webkit.org/show_bug.cgi?id=14442
Summary
Adding a stop with value 1.0 to a gradient that has already been used has no ...
Adam Roben (:aroben)
Reported
2007-06-27 22:14:59 PDT
Adding a stop with value 1.0 to a gradient that has already been used has no effect. This is because CanvasGradient::findStop (
http://trac.webkit.org/projects/webkit/browser/trunk/WebCore/html/CanvasGradient.cpp#L164
) adds stops with values 0.0 and 1.0 to the gradient if they aren't already present. A stop added with a value of 1.0 after findStep has been called will have no effect because it will be sorted after the one findStop added.
Attachments
testcase
(536 bytes, text/html)
2007-06-27 22:15 PDT
,
Adam Roben (:aroben)
no flags
Details
Firefox vs. Safari rendering
(22.40 KB, image/png)
2007-06-27 22:15 PDT
,
Adam Roben (:aroben)
no flags
Details
First attempt
(10.74 KB, patch)
2007-07-04 13:43 PDT
,
Rob Buis
darin
: review-
Details
Formatted Diff
Diff
New attempt
(10.81 KB, patch)
2007-07-05 01:03 PDT
,
Rob Buis
aroben
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1
2007-06-27 22:15:20 PDT
Created
attachment 15283
[details]
testcase
Adam Roben (:aroben)
Comment 2
2007-06-27 22:15:44 PDT
Created
attachment 15284
[details]
Firefox vs. Safari rendering
Rob Buis
Comment 3
2007-07-04 13:43:36 PDT
Created
attachment 15390
[details]
First attempt This one should work, though I had to do it quickly so it needs good reviewing. Cheers, Rob.
Darin Adler
Comment 4
2007-07-05 00:14:55 PDT
Comment on
attachment 15390
[details]
First attempt Seems like a good idea. But I don't think this is right yes. I don't think that findStop will return -1 for values that are less than the first stop; we need a test case that covers that. Also, I don't think the special cases in CanvasGradient::getColor for <= 0 and >= 1 will work properly, since they assume that <= 0 and >= 1 are the first and last stop. In fact, I suspect that was already broken since the sorting is done inside findStop, and there's no guarantee it's called in those cases.
Rob Buis
Comment 5
2007-07-05 01:03:14 PDT
Created
attachment 15394
[details]
New attempt The spec says: "Before the first stop, the color must be the color of the first stop. After the last stop, the color must be the color of the last stop." It seems like we did that wrong, FF seems to confirm it. This patch corrects that and fixes the regression. Cheers, Rob.
Adam Roben (:aroben)
Comment 6
2007-07-06 01:52:08 PDT
Comment on
attachment 15394
[details]
New attempt Add an ASSERT(m_stopsSorted) to the beginning of CanvasGradient::findStop otherwise, r=me
Rob Buis
Comment 7
2007-07-06 02:02:24 PDT
Landed in
r24048
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug