Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(263)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1075413002: Explicitly enforce values size in feColorMatrix. (Closed)

Created:
5 years, 8 months ago by Stephen Chennney
Modified:
5 years, 8 months ago
Reviewers:
pdr., Stephen White
CC:
blink-reviews, Rik, danakj, Dominik Röttsches, dshwang, krit, ed+blinkwatch_opera.com, f(malita), gyuyoung2, jbroman, Justin Novosad, kouhei+svg_chromium.org, pdr+graphicswatchlist_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Explicitly enforce values size in feColorMatrix. R=senorblanco@chromium.org BUG=468519 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=193571

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -2 lines) Patch
M Source/core/svg/SVGFEColorMatrixElement.cpp View 1 chunk +6 lines, -2 lines 1 comment Download
M Source/platform/graphics/filters/FEColorMatrix.cpp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 12 (2 generated)
Stephen Chennney
Simplest fix I can think of.
5 years, 8 months ago (2015-04-10 20:34:44 UTC) #1
Stephen White
If the bots are happy, I'm happy. LGTM
5 years, 8 months ago (2015-04-10 20:36:24 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1075413002/1
5 years, 8 months ago (2015-04-10 20:42:05 UTC) #4
pdr.
https://codereview.chromium.org/1075413002/diff/1/Source/core/svg/SVGFEColorMatrixElement.cpp File Source/core/svg/SVGFEColorMatrixElement.cpp (right): https://codereview.chromium.org/1075413002/diff/1/Source/core/svg/SVGFEColorMatrixElement.cpp#newcode82 Source/core/svg/SVGFEColorMatrixElement.cpp:82: if (values.size() == 20) In a followup could you ...
5 years, 8 months ago (2015-04-10 20:59:40 UTC) #6
Stephen Chennney
On 2015/04/10 20:59:40, pdr wrote: > https://codereview.chromium.org/1075413002/diff/1/Source/core/svg/SVGFEColorMatrixElement.cpp > File Source/core/svg/SVGFEColorMatrixElement.cpp (right): > > https://codereview.chromium.org/1075413002/diff/1/Source/core/svg/SVGFEColorMatrixElement.cpp#newcode82 > ...
5 years, 8 months ago (2015-04-10 21:28:21 UTC) #7
pdr.
On 2015/04/10 at 21:28:21, schenney wrote: > On 2015/04/10 20:59:40, pdr wrote: > > https://codereview.chromium.org/1075413002/diff/1/Source/core/svg/SVGFEColorMatrixElement.cpp ...
5 years, 8 months ago (2015-04-10 21:29:11 UTC) #8
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://src.chromium.org/viewvc/blink?view=rev&revision=193571
5 years, 8 months ago (2015-04-10 22:35:31 UTC) #9
Erik Dahlström (inactive)
Hmm... is this really correct? Spec: http://www.w3.org/TR/SVG11/filters.html#feColorMatrixValuesAttribute If type="saturate" or type="hueRotate" the correct number of ...
5 years, 8 months ago (2015-04-13 11:01:08 UTC) #10
fs
On 2015/04/13 11:01:08, Erik Dahlström wrote: > Hmm... is this really correct? > > Spec: ...
5 years, 8 months ago (2015-04-13 11:25:30 UTC) #11
Stephen Chennney
5 years, 8 months ago (2015-04-13 18:03:58 UTC) #12
Message was sent while issue was closed. 
On 2015/04/13 11:25:30, fs wrote:
> On 2015/04/13 11:01:08, Erik Dahlström wrote:
> > Hmm... is this really correct?
> > 
> > Spec: http://www.w3.org/TR/SVG11/filters.html#feColorMatrixValuesAttribute
> > 
> > If type="saturate" or type="hueRotate" the correct number of values in the
> > 'values' attribute should be 1, not 20.
> 
> Yes, seems what you'd want here is something like what's in build() (which
> factors in 'type').

Oops. I'll look into it.

Powered by Google App Engine
This is Rietveld 408576698