third_party/WebKit/LayoutTests/svg/custom/duplicate-ids-removal.html - Issue 2722543002: Improve handling of duplicate id's for SVG resources

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: third_party/WebKit/LayoutTests/svg/custom/duplicate-ids-removal.html

Issue 2722543002: Improve handling of duplicate id's for SVG resources (Closed)

Patch Set: Created 3 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« third_party/WebKit/LayoutTests/svg/custom/duplicate-ids-addition.html ('K') | « third_party/WebKit/LayoutTests/svg/custom/duplicate-ids-id-mutation-expected.html ('k') | third_party/WebKit/LayoutTests/svg/custom/duplicate-ids-removal-expected.html » ('j') | third_party/WebKit/Source/core/svg/SVGTreeScopeResources.cpp » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <script src="../../resources/run-after-layout-and-paint.js"></script>

	3 <svg>

	4 <linearGradient id="f">

	5 <stop stop-color="green"/>

	6 </linearGradient>

	7 <linearGradient id="f">

	8 <stop stop-color="red"/>

	9 </linearGradient>

	10 <rect width="100" height="100" fill="url(#f) orange"/>

	11 </svg>

	12 <script>

	13 runAfterLayoutAndPaint(function() {

	14 document.querySelectorAll('lineargradient')[1].remove();
	Stephen Chennney 2017/02/27 16:21:22 Another test comes to mind. If you remove the firs Another test comes to mind. If you remove the first one we should be sure to recompute and pick up the second one (which will now be the first one). fs 2017/02/27 17:47:30 I wrote this test, but it's currently failing (so Show quoted text On 2017/02/27 at 16:21:22, Stephen Chennney wrote: > Another test comes to mind. If you remove the first one we should be sure to recompute and pick up the second one (which will now be the first one). I wrote this test, but it's currently failing (so nice catch!) so I need to debug this some more. (I suspect it might be related to the "currentResource == resource" check in removeResource, which would be "doh!" I suspect...) fs 2017/02/27 19:11:25 I realize that that test likely worked "by acciden Show quoted text On 2017/02/27 at 17:47:30, fs wrote: > On 2017/02/27 at 16:21:22, Stephen Chennney wrote: > > Another test comes to mind. If you remove the first one we should be sure to recompute and pick up the second one (which will now be the first one). > > I wrote this test, but it's currently failing (so nice catch!) so I need to debug this some more. (I suspect it might be related to the "currentResource == resource" check in removeResource, which would be "doh!" I suspect...) I realize that that test likely worked "by accident" before because the second gradient would be the one in the map after style-recalc/layout. (Easily observed when running the test because it does flash red -> green...) I'll see if can do something about without adding too much complexity though...
	15 }, true);

	16 </script>

OLD	NEW