Bug 15495 - SVGViewSpec DOM bindings aka SVGSVGElement.currentView is unimplemented
Summary: SVGViewSpec DOM bindings aka SVGSVGElement.currentView is unimplemented
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
: 15503 (view as bug list)
Depends on: 15504
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-13 22:56 PDT by Eric Seidel
Modified: 2012-05-29 01:42 PDT (History)
12 users (show)

See Also:


Attachments
test "standard" behavior (download to LayoutTests/svg/dom) (3.27 KB, application/xhtml+xml)
2007-10-13 22:56 PDT, Eric Seidel
no flags Details
Several tests for LayoutTests/svg/dom (some are incomplete) (26.12 KB, application/zip)
2007-10-13 22:59 PDT, Eric Seidel
no flags Details
incomplete patch (51.62 KB, patch)
2007-10-13 22:59 PDT, Eric Seidel
no flags Details | Formatted Diff | Diff
Patch (224.44 KB, patch)
2012-05-25 03:35 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v2 (239.43 KB, patch)
2012-05-25 04:37 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (548.57 KB, application/zip)
2012-05-25 06:25 PDT, WebKit Review Bot
no flags Details
Patch v3 (238.12 KB, patch)
2012-05-26 02:45 PDT, Nikolas Zimmermann
rwlbuis: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel 2007-10-13 22:56:15 PDT
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.
Comment 1 Eric Seidel 2007-10-13 22:56:57 PDT
Created attachment 16655 [details]
test "standard" behavior (download to LayoutTests/svg/dom)
Comment 2 Eric Seidel 2007-10-13 22:59:07 PDT
Created attachment 16656 [details]
Several tests for LayoutTests/svg/dom (some are incomplete)
Comment 3 Eric Seidel 2007-10-13 22:59:49 PDT
Created attachment 16657 [details]
incomplete patch
Comment 4 Eric Seidel 2007-10-18 00:41:36 PDT
A chunk of this was landed as part of bug 15504 (the fix to the parser to prevent crashing).
Comment 5 Nikolas Zimmermann 2012-05-19 17:21:24 PDT
*** Bug 15503 has been marked as a duplicate of this bug. ***
Comment 6 Nikolas Zimmermann 2012-05-25 03:35:24 PDT
Created attachment 144033 [details]
Patch
Comment 7 Gustavo Noronha (kov) 2012-05-25 03:50:53 PDT
Comment on attachment 144033 [details]
Patch

Attachment 144033 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12791497
Comment 8 Nikolas Zimmermann 2012-05-25 04:37:23 PDT
Created attachment 144045 [details]
Patch v2
Comment 9 WebKit Review Bot 2012-05-25 06:25:21 PDT
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
Comment 10 WebKit Review Bot 2012-05-25 06:25:27 PDT
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
Comment 11 Rob Buis 2012-05-25 07:49:30 PDT
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?
Comment 12 Nikolas Zimmermann 2012-05-26 02:45:00 PDT
Created attachment 144200 [details]
Patch v3
Comment 13 Nikolas Zimmermann 2012-05-26 02:49:04 PDT
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.
Comment 14 Rob Buis 2012-05-26 14:10:05 PDT
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.
Comment 15 Nikolas Zimmermann 2012-05-29 01:32:54 PDT
(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.
Comment 16 Nikolas Zimmermann 2012-05-29 01:42:23 PDT
Committed r118735: <http://trac.webkit.org/changeset/118735>