Bug 15495 – SVGViewSpec DOM bindings aka SVGSVGElement.currentView is unimplemented

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Eric Seidel (no email)]

SVGViewSpec DOM bindings aka SVGSVGElement.currentView is unimplemented

Unfortunately it's so under-specified in the spec that it's hard to implement.  I've raised:
http://www.w3.org/Bugs/Public/show_bug.cgi?id=5191
with the SVG working group.  Hopefully more information will come of that.

I'm attaching my partial patch and a couple test cases and moving on to other things.  We'll come back to this someday.

[Eric Seidel (no email)]

Created attachment 16655 [details]
test "standard" behavior (download to LayoutTests/svg/dom)

[Eric Seidel (no email)]

Created attachment 16656 [details]
Several tests for LayoutTests/svg/dom (some are incomplete)

[Eric Seidel (no email)]

Created attachment 16657 [details]
incomplete patch

[Eric Seidel (no email)]

A chunk of this was landed as part of bug 15504 (the fix to the parser to prevent crashing).

[Nikolas Zimmermann]

*** Bug 15503 has been marked as a duplicate of this bug. ***

[Nikolas Zimmermann]

Created attachment 144033 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 144033 [details]
Patch

Attachment 144033 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12791497

[Nikolas Zimmermann]

Created attachment 144045 [details]
Patch v2

[WebKit Review Bot]

Comment on attachment 144045 [details]
Patch v2

Attachment 144045 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12829005

New failing tests:
svg/dom/viewspec-parser.html

[WebKit Review Bot]

Created attachment 144059 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Rob Buis]

Comment on attachment 144045 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=144045&action=review

It is unfortunate that the patch is so big, but according to Nikolas it is impractical to split it up. Code looks good, can the failing test be fixed for non OS X?

> Source/WebCore/ChangeLog:9
> +        - SVGViewSpec and all of its concents should be read-only. Enforce that and test it.

contents

> Source/WebCore/ChangeLog:10
> +          Add a new enum to SVGPropertyInfo so that each SVGAnimatedProperty knows if its content is suppoed to be read-write or read-only.

supposed

> LayoutTests/svg/dom/resources/viewspec-parser.svg:4
> +</svg>

Is this a good file name?

> LayoutTests/svg/dom/viewspec-parser.html:196
> +</html>

What changed in this file?

[Nikolas Zimmermann]

Created attachment 144200 [details]
Patch v3

[Nikolas Zimmermann]

Fixed all typos. Changed the viewspec-parser.html to avoid the printf() differences, regarding \0 termination - I hope the new version passes chromium as well.

> It is unfortunate that the patch is so big, but according to Nikolas it is impractical to split it up. Code looks good, can the failing test be fixed for non OS X?
Exposing SVGViewSpec requires all the svg/properties changes - but they can't be done separated, as the current form of SVGViewSpec wouldn't compile then :( I hope this is okay.

> > LayoutTests/svg/dom/resources/viewspec-parser.svg:4
> > +</svg>
> 
> Is this a good file name?
Renamed to viewspec-target.svg, there's no parsing involved.
 
> > LayoutTests/svg/dom/viewspec-parser.html:196
> > +</html>
> 
> What changed in this file?
Best to compare with two views, manually, sorry. webkit-patch upload produces this style of patch.

[Rob Buis]

Comment on attachment 144200 [details]
Patch v3

View in context: https://bugs.webkit.org/attachment.cgi?id=144200&action=review

Looks good.

> LayoutTests/platform/mac/svg/custom/linking-a-03-b-all-expected.txt:23
>        RenderSVGRect {rect} at (0,0) size 480x360 [stroke={[type=SOLID] [color=#000000]}] [x=1.00] [y=1.00] [width=478.00] [height=358.00]

I wonder, can this test be made a reftest? Would save some rebaselining. I leave it up to you if you want to do that before landing this patch, assuming it is possible at all.

[Nikolas Zimmermann]

(In reply to comment #14)
> (From update of attachment 144200 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=144200&action=review
> 
> Looks good.
> 
> > LayoutTests/platform/mac/svg/custom/linking-a-03-b-all-expected.txt:23
> >        RenderSVGRect {rect} at (0,0) size 480x360 [stroke={[type=SOLID] [color=#000000]}] [x=1.00] [y=1.00] [width=478.00] [height=358.00]
> 
> I wonder, can this test be made a reftest? Would save some rebaselining. I leave it up to you if you want to do that before landing this patch, assuming it is possible at all.

Great idea, I'll remove all platform specific results of this test and make it a reftest.
The other linking-* tests can be converted easily as well, but I'll leave that for another patch.

[Nikolas Zimmermann]

Committed r118735: <http://trac.webkit.org/changeset/118735>