[cairo] [PATCH] Enable links to image files in SVG backend
alex.shulgin at gmail.com
Wed Jan 20 04:08:51 PST 2010
On Wed, Jan 20, 2010 at 13:25, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> Comments inline.
> On Wed, 20 Jan 2010 12:56:44 +0200, Alexander Shulgin <alex.shulgin at gmail.com> wrote:
>> OK, now I've got this:
>> diff --git a/doc/public/cairo-sections.txt b/doc/public/cairo-sections.txt
>> index ff89db7..1e45c59 100644
>> --- a/doc/public/cairo-sections.txt
>> +++ b/doc/public/cairo-sections.txt
>> @@ -176,6 +176,8 @@ cairo_svg_version_to_string
> I leave others to debate the merits of accepting a non-rfc3986 encoded
> URI, and whether it is wise for Cairo just to mandate use of
> g_filename_to_uri (or whatever) for local files.
I don't think forcing clients depend on glib or any other third-party
lib for this is in any way friendly. And yet again, I've come to this
problem from Ruby, so this might cause even more trouble for clients.
>> + * _cairo_svg_surface_uri_encode:
>> + *
>> + * Percent-encode the URI string. Percent sign itself is not in
>> + * allowed set so it gets encoded.
>> + *
>> + * RFC: http://tools.ietf.org/html/rfc3986
>> + **/
> This is not svg specific, so if we keep the uri encoding, we should create
> a cairo-uri.c.
Agreed, so lets decide whether we keep this first.
>> + *enc = malloc (length*3 + 1 /* to fit nul-byte from last snprintf */);
> We need to do pessimistic integer overflow checks here. So use
> _cairo_malloc_ab_plus_c(length, 3, 1); it's a pain but better than a
> security exploit.
OK, no problem.
>> + if (!*enc)
>> + return CAIRO_STATUS_NO_MEMORY;
> Please use return _cairo_error (CAIRO_STATUS_NO_MEMORY);
> The _cairo_error() gives us a breakpoint on the origin of an error,
> absolutely invaluable when tracking down library bugs.
Oh, this is interesting. Never thought something like this is
possible, but it's so simple. :)
>> diff --git a/src/cairo.h b/src/cairo.h
>> index 8be8043..16cafaf 100644
>> --- a/src/cairo.h
>> +++ b/src/cairo.h
>> @@ -2042,6 +2042,8 @@ cairo_surface_set_user_data (cairo_surface_t *surface,
>> #define CAIRO_MIME_TYPE_JPEG "image/jpeg"
>> #define CAIRO_MIME_TYPE_PNG "image/png"
>> #define CAIRO_MIME_TYPE_JP2 "image/jp2"
>> +#define CAIRO_MIME_TYPE_XURI "text/x-uri"
>> +#define CAIRO_MIME_TYPE_XURI_ENCODED "text/x-uri-encoded"
> If we were to accepted both XURI and XURI_ENCODED, the one we want people
> to use most often should have the shorter name, ie XURI and
> XURI_NON_ENCODED. What's the justification for the non-encoded XURI
> again? ;-)
The more I think about it, the more I'm in favor of renaming these to
XFILENAME and XURI (or XURI_ENCODED if you want it to be harder to
type (; ).
Thing is, producing valid URIs from filenames is not really trivial
(at least it's not what client code is going to bother with). So,
when user has a good old filename, he goes with
CAIRO_MIME_TYPE_XFILENAME, and if we see this in SVG backend, we do
%-encode it. Other backends, if they handle
CAIRO_MIME_TYPE_XFILENAME, might need some encoding for this or might
When user knows he has a valid URI he'll use CAIRO_MIME_TYPE_XURI, and
if we see this in SVG backend we won't break things for the user as
we're not going to encode it one more time.
>> diff --git a/src/uri-allowed-set-lookup-table.inc
> I'm resistant to the first .inc file, it's not a convention in wide usage.
> In this case, I would just include it directly in cairo-uri.c with the
> generator in a comment (which would be even more useful if was directly
> executable, for example a python script ;-).
OK, so let's wait for decision to keep the URI stuff in cairo.
>> Is this looking any good? :)
> Yes. :-)
Thanks for your comments!
More information about the cairo