Bug 5996

Summary: SVG <view> is unimplemented
Product: WebKit Reporter: Eric Seidel (no email) <eric>
Component: SVGAssignee: Rob Buis <rwlbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: ian
Priority: P4    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.w3.org/TR/SVG11/linking.html#ViewElement
Attachments:
Description Flags
rough outline patch (doens't actually work)
none
Work in progress
none
Complete code patch, no tests yet
none
Now with testcases zimmermann: review+

Description Eric Seidel (no email) 2005-12-07 21:05:18 PST
SVG <view> is unimplemented

It actually doesn't look that hard.  It's the initial zoom/pan to apply to the document when activating a 
link, that's all.  Shouldn't be very difficult to build up the DOM objects, and then it's just a bit of wiring into 
the linking system.
Comment 1 Alexander Kellett 2005-12-10 15:26:54 PST
possibly useful test

LayoutTests/svg/W3C-SVG-1.1/resources/linking-uri-01-b.svg
Comment 2 Eric Seidel (no email) 2006-08-25 02:34:27 PDT
I'm sorta surprised no one has jumped on this yet.  This would be (IMO) a fun, easy fix to make.
Comment 3 Eric Seidel (no email) 2006-08-25 04:57:06 PDT
Ok, maybe this isn't so super easy to fix.
Comment 4 Eric Seidel (no email) 2006-08-25 04:57:45 PDT
Created attachment 10216 [details]
rough outline patch (doens't actually work)
Comment 5 Eric Seidel (no email) 2007-02-04 03:29:48 PST
Here is another test of view fragment urls:
http://www.w3.org/Graphics/SVG/Test/20061213/htmlEmbedHarness/full-linking-a-03-b.html
Comment 7 Eric Seidel (no email) 2007-06-12 08:42:05 PDT
Rob had expressed interest in fixing this.
Comment 8 Rob Buis 2007-06-30 15:29:02 PDT
(In reply to comment #7)
> Rob had expressed interest in fixing this.
> 

and still does.
Cheers,

Rob.
Comment 9 Rob Buis 2007-07-03 03:40:49 PDT
Created attachment 15361 [details]
Work in progress

This patch is almost complete, it is based on Eric's patch. There are some bugs to fix and ofcourse some tests to create, but from my simple testing it seems to work fine. I hope to have something reviewable soon :)
Cheers,

Rob.
Comment 10 Nikolas Zimmermann 2007-07-03 07:12:37 PDT
(In reply to comment #9)
> Created an attachment (id=15361) [edit]
> Work in progress
> 
> This patch is almost complete, it is based on Eric's patch. There are some bugs
> to fix and ofcourse some tests to create, but from my simple testing it seems
> to work fine. I hope to have something reviewable soon :)
> Cheers,
> 
> Rob.

Excellent work! Just had a quick look, can't do real reviewing atm - as I'm blocked by
exams (Saturday!). I love the RenderSVGRoot code removals though...

What bugs are still existant? Did you file follow up bugs already for these?

Greetings,
Niko
Comment 11 Rob Buis 2007-07-04 02:56:33 PDT
Created attachment 15381 [details]
Complete code patch, no tests yet

As the description says, code is complete, I am not sure how to tests yet though. Obviously the manual tests work. If it can be automated I need to find some examples of how to do tests where a link is followed. The code part can be reviewed now though I think.
Cheers,

Rob.
Comment 12 Oliver Hunt 2007-07-04 03:42:28 PDT
Comment on attachment 15381 [details]
Complete code patch, no tests yet

the "static const UChar svgViewSpec[] = {'s','v','g','V', 'i', 'e', 'w'};" declarations may trigger our initialiser logic, so you may ned to turn them into functions wrapping scoped globals...

Other thasn that it looks fine, but this wasn't as thorough review as really necessary
Comment 13 Rob Buis 2007-07-13 13:16:10 PDT
Created attachment 15504 [details]
Now with testcases

A first shot at some testcases.
Cheers,

Rob.
Comment 14 Nikolas Zimmermann 2007-07-16 02:55:45 PDT
(In reply to comment #13)
> Created an attachment (id=15504) [edit]
> Now with testcases
> 
> A first shot at some testcases.
> Cheers,
> 
> Rob.

Hey Rob,

I'm very tempted to r+ it now. Did you think about Oliver's issue?
I think that's the last thing holding it back from r+.

One last thing:
// FIXME: All this setup should be done after attributesChanged, not here.
This comment is outdated in SVGSVGElement::createRenderer.
Maybe just move it to the approriate place?

Greetings,
Niko
Comment 15 Nikolas Zimmermann 2007-07-16 11:41:42 PDT
Comment on attachment 15504 [details]
Now with testcases

Good job.
Comment 16 Rob Buis 2007-07-16 21:10:40 PDT
Landed in r24349.