Bug 5996 - SVG <view> is unimplemented
Summary: SVG <view> is unimplemented
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P4 Normal
Assignee: Rob Buis
URL: http://www.w3.org/TR/SVG11/linking.ht...
Keywords:
Depends on:
Blocks:
 
Reported: 2005-12-07 21:05 PST by Eric Seidel (no email)
Modified: 2007-07-16 21:10 PDT (History)
1 user (show)

See Also:


Attachments
rough outline patch (doens't actually work) (4.44 KB, patch)
2006-08-25 04:57 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Work in progress (38.04 KB, patch)
2007-07-03 03:40 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Complete code patch, no tests yet (60.48 KB, patch)
2007-07-04 02:56 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Now with testcases (258.74 KB, patch)
2007-07-13 13:16 PDT, Rob Buis
zimmermann: 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 (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.