Summary: | Adding a stop with value 1.0 to a gradient that has already been used has no effect | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | ||||||||||||
Priority: | P2 | ||||||||||||
Version: | 523.x (Safari 3) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Adam Roben (:aroben)
2007-06-27 22:14:59 PDT
Created attachment 15283 [details]
testcase
Created attachment 15284 [details]
Firefox vs. Safari rendering
Created attachment 15390 [details]
First attempt
This one should work, though I had to do it quickly so it needs good reviewing.
Cheers,
Rob.
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.
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.
Comment on attachment 15394 [details]
New attempt
Add an ASSERT(m_stopsSorted) to the beginning of CanvasGradient::findStop
otherwise, r=me
|