[Webkit-unassigned] [Bug 136903] changing SVG <defs> does not update SVG element.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=136903

Sylvain Galineau <galineau at adobe.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #242353|0                           |1
        is obsolete|                            |

--- Comment #14 from Sylvain Galineau <galineau at adobe.com> ---
Created attachment 242361
  --> https://bugs.webkit.org/attachment.cgi?id=242361&action=review
[WIP] fix.patch


> You need to fill in the OOPS! lines here - at least the description and the bug.

Yes, my bad. I missed the LayoutTests ChangeLog :( Fixed.

>For the s/dom/dom-static-property-for-in-iteration-expected.txt line, 
>you should probably explain why this changed. e.g. You moved innerHTML 
>and outerHTML onto Element.

Done.

>And I think this line is wrong:

>platform/mac/svg/in-html/svg-inner-html-expected.png: Added

>Didn't you remove that image now that it is a text test?

This was intentional. The reason: when I started working on this I was quickly able to get innerHTML to return the proper string even though it failed to render properly in an HTML document. So I think we want to test that both 1) innerHTML does the right thing DOM-wise - here I'm checking the innerHTML of the <pattern> added through defs.innerHTML - and 2) that the proper renderer gets invoked to make the rectangle green. Is there a better way? Do I also need a .svg version of the test?

>> LayoutTests/svg/css/svg-attribute-length-parsing-expected.txt:2
>> +Test
>>  Test CSS parsing on SVG presentation attributes.

>Was this expected to change? I don't see the actual test changing.

The test is unchanged, but the expected result is different. The test code in svg-attribute-length-parsin.js writes 'Test' to the innerHTML of an SVG element it creates. This was a no-op in the past but obviously no longer is; thus I believe this new expected.txt to be the correct one. Alternatively, I could fix the JS code to not add that innerHTML 'Test' string but I wasn't sure if this might have other side-effects.

> -        No new tests needed.
> +        The latter part was ported over from Blink. 
>  

>Something really crazy is going on here. I suggest you revert all your changes to >ChangeLog, update, then call prepare-ChangeLog again and add your content back.

Something really crazy seems the proper technical term. Couldn't make sense of it either so reverted and added content back in.


>Nit: Period at end of sentence.

Done.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141202/da8aa06c/attachment-0002.html>