Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1817623004:
When the SVG element is 'visible', the SVG element is not shown (Closed)
|
Created:
4 years, 9 months ago by hyunjunekim2 Modified:
4 years, 9 months ago CC:
blink-reviews, blink-reviews-paint_chromium.org, chromium-reviews, dshwang, slimming-paint-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionWhen the SVG element is 'visible', the SVG element is not shown
Can't display the svg element that is not root and has 'visible'.
When paint the root element that is hidden, does not give a chance
to the children whether paint or not.
Because ignore to paint hidden <svg> element except 'border-radius'
and if children of hidden <svg> element have 'visibility=visible',
paint it.
BUG=590153
Committed: https://crrev.com/776c27e916f27261eff437954efe0d79b6195f30
Cr-Commit-Position: refs/heads/master@{#383252}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #Patch Set 4 : #Patch Set 5 : #Patch Set 6 : #Patch Set 7 : #
Total comments: 8
Patch Set 8 : #Patch Set 9 : #Patch Set 10 : #Patch Set 11 : #Patch Set 12 : #Patch Set 13 : #
Total comments: 12
Patch Set 14 : #Patch Set 15 : #Patch Set 16 : #Patch Set 17 : When hidden, ignore selection tint. #Patch Set 18 : #
Total comments: 2
Patch Set 19 : #Messages
Total messages: 47 (24 generated)
Description was changed from ========== Issue 590153 BUG=590153 ========== to ========== Issue 590153 BUG=590153 ==========
Description was changed from ========== Issue 590153 BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown ... BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown ... BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. So If it's the SVG root, do not check visibility. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. So If it's the SVG root, do not check visibility. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. So if it's the SVG root, do not check visibility. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. So if it's the SVG root, do not check visibility. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. Only ignore painting ReplacedPainter. BUG=590153 ==========
hyunjune.kim@samsung.com changed reviewers: + fs@opera.com, pdr@chromium.org
fs, pdr Could you check PS7 and give me advice? Thank you.
https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element-expected.html (right): https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element-expected.html:2: <div> The reference could just be a 100x100 div with a green background(-color)? (If the <text> is removed.) https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element.html (right): https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element.html:7: <div class="hidden"> Could use inline style here as well. https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element.html:11: <text x="35" y="55" visibility="visible">Pass</text> Any specific case that including this <text> covers, or would the <rect> be enough? https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:29: if (m_layoutReplaced.isSVGRoot() && m_layoutReplaced.style()->visibility() != VISIBLE) I _think_ that clipping from 'overflow' should still be applied, i.e the following should still clip: https://jsfiddle.net/jzmdr360/1/ https://jsfiddle.net/vzn8r10o/ So maybe the check for visibility == hidden needs to be hoisted out of shouldPaint(...).
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. Only ignore painting ReplacedPainter. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. So ignore painting ReplacedPainter. BUG=590153 ==========
fs, Could you check PS13 and give me advice? https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element-expected.html (right): https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element-expected.html:2: <div> On 2016/03/21 16:07:58, fs wrote: > The reference could just be a 100x100 div with a green background(-color)? (If > the <text> is removed.) Done. https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element.html (right): https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element.html:7: <div class="hidden"> On 2016/03/21 16:07:58, fs wrote: > Could use inline style here as well. Done. https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/svg/custom/visibility-enable-on-svg-element.html:11: <text x="35" y="55" visibility="visible">Pass</text> On 2016/03/21 16:07:58, fs wrote: > Any specific case that including this <text> covers, or would the <rect> be > enough? Done. https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:29: if (m_layoutReplaced.isSVGRoot() && m_layoutReplaced.style()->visibility() != VISIBLE) Could you checkPS13? Added more tests. Thank you.
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. So ignore painting ReplacedPainter. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. Just ignore painting ReplacedPainter. BUG=590153 ==========
https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:33: if (m_layoutReplaced.style()->visibility() != HIDDEN && m_layoutReplaced.hasBoxDecorationBackground() && (paintInfo.phase == PaintPhaseForeground || paintInfo.phase == PaintPhaseSelection)) Ought to be == VISIBLE here (to handle 'collapse' which should be the same as 'hidden' in this case.) https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); Should we paint the selection tint? Given it's a foreground thing, probably not. https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:105: if (!m_layoutReplaced.isSVGRoot() && m_layoutReplaced.style()->visibility() != VISIBLE) Document the new part of the condition.
fs, Could you check this patch? and give me advice. Thank you :) https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:33: if (m_layoutReplaced.style()->visibility() != HIDDEN && m_layoutReplaced.hasBoxDecorationBackground() && (paintInfo.phase == PaintPhaseForeground || paintInfo.phase == PaintPhaseSelection)) On 2016/03/23 14:44:23, fs wrote: > Ought to be == VISIBLE here (to handle 'collapse' which should be the same as > 'hidden' in this case.) Done. https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); On 2016/03/23 14:44:23, fs wrote: > Should we paint the selection tint? Given it's a foreground thing, probably not. I guess that we paint this. In such cases, the will should be a tint. <!DOCTYPE html> <style type="text/css"> img { width: 160px; height: 100px; } </style> <img src="data:image/svg+xml, <svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16' visibility='hidden'> <rect x='1' y='1' width='14' height='14' fill='lime' visibility='visible'/> </svg>"> https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:105: if (!m_layoutReplaced.isSVGRoot() && m_layoutReplaced.style()->visibility() != VISIBLE) On 2016/03/23 14:44:23, fs wrote: > Document the new part of the condition. Added the comment.
It made the wrong case. https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); fs, It's different case.
fs, Could you check PS17? https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); On the SVG, when images, It can be selection tint. then ignore visible option.
question. :) https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); fs, I have question. Is there a case that tint on the svg document?
fs, I looked at. I guess that it's not solution. Let a little more. and i re-apply review to you.
https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); On 2016/03/24 at 13:12:24, hyunjunekim2 wrote: > fs, > I have question. Is there a case that tint on the svg document? You might be able to set one explicitly as a Range (via window.selection)
On 2016/03/24 13:47:35, fs wrote: > https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): > > https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool > drawSelectionTint = paintInfo.phase == PaintPhaseForeground && > m_layoutReplaced.getSelectionState() != SelectionNone && > !paintInfo.isPrinting(); > On 2016/03/24 at 13:12:24, hyunjunekim2 wrote: > > fs, > > I have question. Is there a case that tint on the svg document? > > You might be able to set one explicitly as a Range (via window.selection) fs, And about BUG=590153, has one more problem. If the element on the <clipPath> is HIDDEN, can not apply clip-path to no-resource element. <div id="container" style="visibility: hidden;"> <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="400" height="300"> <clipPath id="clip"> <rect x="0" y="0" width="400" height="300"></rect> </clipPath> <rect x="20" y="10" width="200" height="100" fill="red" visibility="visible" clip-path="url(#clip)"></rect> </svg> </div>
On 2016/03/24 at 14:04:22, hyunjune.kim wrote: > On 2016/03/24 13:47:35, fs wrote: > > https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... > > File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): > > > > https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... > > third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool > > drawSelectionTint = paintInfo.phase == PaintPhaseForeground && > > m_layoutReplaced.getSelectionState() != SelectionNone && > > !paintInfo.isPrinting(); > > On 2016/03/24 at 13:12:24, hyunjunekim2 wrote: > > > fs, > > > I have question. Is there a case that tint on the svg document? > > > > You might be able to set one explicitly as a Range (via window.selection) > fs, > And about BUG=590153, has one more problem. > If the element on the <clipPath> is HIDDEN, can not apply clip-path to no-resource element. > > <div id="container" style="visibility: hidden;"> > <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="400" height="300"> > <clipPath id="clip"> > <rect x="0" y="0" width="400" height="300"></rect> > </clipPath> > <rect x="20" y="10" width="200" height="100" fill="red" visibility="visible" clip-path="url(#clip)"></rect> > </svg> > </div> Yes, that's better to fix in another CL (the reason is probably that 'visibility' is respected for elements within <clipPath> even though it shouldn't be.)
Could you check PS 18? >Yes, that's better to fix in another CL (the reason is probably that 'visibility' is respected for elements within <clipPath> even though it shouldn't be.) Ok, After solved this case, and i will fix it about resource element. https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); fs, We can't select <svg> element. I ran this test. but just selected <text>. So i guess that don't care wether visible or not. <!DOCTYPE HTML> <div style="visibility: hidden;"> <svg id="s1" style="background: red;"> <rect id="r1" x="0" y="0" width="100" height="100" fill="green" visibility="visible"></rect> <text x="20" y="20" visibility="visible">hi</text> </svg> </div> <script> var range = document.createRange(); range.selectNode(window.document.documentElement); var selection = window.getSelection(); selection.removeAllRanges(); selection.addRange(range); console.log(selection); </script>
On 2016/03/24 15:31:34, hyunjunekim2 wrote: > Could you check PS 18? > > >Yes, that's better to fix in another CL (the reason is probably that > 'visibility' is respected for elements within <clipPath> even though it > shouldn't be.) > > Ok, After solved this case, and i will fix it about resource element. > > https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): > > https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool > drawSelectionTint = paintInfo.phase == PaintPhaseForeground && > m_layoutReplaced.getSelectionState() != SelectionNone && > !paintInfo.isPrinting(); > fs, > We can't select <svg> element. > I ran this test. but just selected <text>. So i guess that don't care whether > visible or not. > > <!DOCTYPE HTML> > <div style="visibility: hidden;"> > <svg id="s1" style="background: red;"> > <rect id="r1" x="0" y="0" width="100" height="100" fill="green" > visibility="visible"></rect> > <text x="20" y="20" visibility="visible">hi</text> > </svg> > </div> > <script> > var range = document.createRange(); > range.selectNode(window.document.documentElement); > var selection = window.getSelection(); > selection.removeAllRanges(); > selection.addRange(range); > console.log(selection); > </script> And selection tint conditions are that phase is PaintPhaseForeground and selection state is not selectionNone and is not printing. But always svg element' selection state is SelectionNone.
LGTM w/ the comment clarified. https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/240001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:87: bool drawSelectionTint = paintInfo.phase == PaintPhaseForeground && m_layoutReplaced.getSelectionState() != SelectionNone && !paintInfo.isPrinting(); On 2016/03/24 at 15:31:33, hyunjunekim2 wrote: > fs, > We can't select <svg> element. > I ran this test. but just selected <text>. So i guess that don't care wether > visible or not. > > <!DOCTYPE HTML> > <div style="visibility: hidden;"> > <svg id="s1" style="background: red;"> > <rect id="r1" x="0" y="0" width="100" height="100" fill="green" visibility="visible"></rect> > <text x="20" y="20" visibility="visible">hi</text> > </svg> > </div> > <script> > var range = document.createRange(); > range.selectNode(window.document.documentElement); > var selection = window.getSelection(); > selection.removeAllRanges(); > selection.addRange(range); > console.log(selection); > </script> Acknowledged. https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:105: // But if it's SVG Root, just ignore this that check whether visible or not. I'd imagine something more like: "But if it's an SVG root, there can be children, so we'll check visibility later."
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. Because when paint the root element, do not give a chance to the children whether paint or not. Just ignore painting ReplacedPainter. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because only ignore to paint hidden <svg> element except 'border-radius' and paint children that are visible. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because only ignore to paint hidden <svg> element except 'border-radius' and paint children that are visible. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because only ignore to paint hidden <svg> element except 'border-radius' and paint children that have 'visibility=visible'. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because only ignore to paint hidden <svg> element except 'border-radius' and paint children that have 'visibility=visible'. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ==========
Also modified description. https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:105: // But if it's SVG Root, just ignore this that check whether visible or not. On 2016/03/24 15:39:03, fs wrote: > I'd imagine something more like: "But if it's an SVG root, there can be > children, so we'll check visibility later." Done.
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, do not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element that is hidden, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ==========
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element that is hidden, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element that is hidden, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ==========
On 2016/03/24 at 16:09:01, hyunjune.kim wrote: > Also modified description. > > https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): > > https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:105: // But if it's SVG Root, just ignore this that check whether visible or not. > On 2016/03/24 15:39:03, fs wrote: > > I'd imagine something more like: "But if it's an SVG root, there can be > > children, so we'll check visibility later." > > Done. Still LGTM
On 2016/03/24 16:22:45, fs wrote: > On 2016/03/24 at 16:09:01, hyunjune.kim wrote: > > Also modified description. > > > > > https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... > > File third_party/WebKit/Source/core/paint/ReplacedPainter.cpp (right): > > > > > https://codereview.chromium.org/1817623004/diff/330001/third_party/WebKit/Sou... > > third_party/WebKit/Source/core/paint/ReplacedPainter.cpp:105: // But if it's > SVG Root, just ignore this that check whether visible or not. > > On 2016/03/24 15:39:03, fs wrote: > > > I'd imagine something more like: "But if it's an SVG root, there can be > > > children, so we'll check visibility later." > > > > Done. > > Still LGTM Thank you, Go ahead.
The CQ bit was checked by hyunjune.kim@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1817623004/350001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1817623004/350001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hyunjune.kim@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1817623004/350001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1817623004/350001
Message was sent while issue was closed.
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element that is hidden, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element that is hidden, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ==========
Message was sent while issue was closed.
Committed patchset #19 (id:350001)
Message was sent while issue was closed.
Description was changed from ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element that is hidden, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 ========== to ========== When the SVG element is 'visible', the SVG element is not shown Can't display the svg element that is not root and has 'visible'. When paint the root element that is hidden, does not give a chance to the children whether paint or not. Because ignore to paint hidden <svg> element except 'border-radius' and if children of hidden <svg> element have 'visibility=visible', paint it. BUG=590153 Committed: https://crrev.com/776c27e916f27261eff437954efe0d79b6195f30 Cr-Commit-Position: refs/heads/master@{#383252} ==========
Message was sent while issue was closed.
Patchset 19 (id:??) landed as https://crrev.com/776c27e916f27261eff437954efe0d79b6195f30 Cr-Commit-Position: refs/heads/master@{#383252} |
