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

Issue 143263011: Convert SVG <text> and descendants to use RenderObject::isChildAllowed (Closed)

Created:
6 years, 11 months ago by fs
Modified:
6 years, 11 months ago
CC:
blink-reviews, krit, bemjb+rendering_chromium.org, dsinclair, zoltan1, eae+blinkwatch, leviw+renderwatch, ed+blinkwatch_opera.com, f(malita), gyuyoung.kim_webkit.org, jchaffraix+rendering, pdr, Stephen Chennney, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Convert SVG <text> and descendants to use RenderObject::isChildAllowed BUG=337607 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165765

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -49 lines) Patch
M Source/core/rendering/svg/RenderSVGTSpan.h View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGTSpan.cpp View 2 chunks +18 lines, -0 lines 1 comment Download
M Source/core/rendering/svg/RenderSVGText.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/RenderSVGTextPath.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/rendering/svg/RenderSVGTextPath.cpp View 2 chunks +15 lines, -0 lines 0 comments Download
M Source/core/svg/SVGAltGlyphElement.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/svg/SVGAltGlyphElement.cpp View 1 chunk +0 lines, -7 lines 0 comments Download
M Source/core/svg/SVGTSpanElement.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/svg/SVGTSpanElement.cpp View 1 chunk +0 lines, -13 lines 0 comments Download
M Source/core/svg/SVGTextElement.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/svg/SVGTextElement.cpp View 1 chunk +0 lines, -14 lines 0 comments Download
M Source/core/svg/SVGTextPathElement.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/svg/SVGTextPathElement.cpp View 1 chunk +0 lines, -10 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
fs
6 years, 11 months ago (2014-01-24 16:46:56 UTC) #1
Stephen Chennney
lgtm
6 years, 11 months ago (2014-01-24 18:07:40 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fs@opera.com/143263011/1
6 years, 11 months ago (2014-01-24 18:08:01 UTC) #3
commit-bot: I haz the power
Change committed as 165765
6 years, 11 months ago (2014-01-24 18:16:23 UTC) #4
pdr.
LGTM as well but we should be on the lookout for security bugs in this ...
6 years, 11 months ago (2014-01-24 18:28:41 UTC) #5
fs
6 years, 11 months ago (2014-01-27 13:24:18 UTC) #6
Message was sent while issue was closed.
On 2014/01/24 18:28:41, pdr wrote:
> LGTM as well but we should be on the lookout for security bugs in this area.
> Disallowed renderer children were not created before but now they will be.

Right.
 
>
https://codereview.chromium.org/143263011/diff/1/Source/core/rendering/svg/Re...
> File Source/core/rendering/svg/RenderSVGTSpan.cpp (right):
> 
>
https://codereview.chromium.org/143263011/diff/1/Source/core/rendering/svg/Re...
> Source/core/rendering/svg/RenderSVGTSpan.cpp:38: {
> Should we add this here?
> 
> if (!RenderSVGInline::isChildAllowed(child, style))
>     return false;
> 
> (Here, and elsewhere. See
>
https://docs.google.com/a/chromium.org/drawings/d/1nayJX5XcclYj_-blbVuakVNjVW...)

I think I'll do this for all cases where it makes sense (which will be most
cases I suppose.) Possibly together with a cleanup/commoning of the predicates.

Powered by Google App Engine
This is Rietveld 408576698