134571 - client-side image maps in showdependencygraph.cgi

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, enhancement, P5)

Description

[John Vandenberg]

Dependency graphs use server-side image maps in order that a user can click on a
bug's node to bring up the bug's details.  When using 'dot' (bug 120537) or the
GraphViz module (bug 134565), the image map data is able to be generated in time
to be included as a client-side image maps.

Comment 1

[Gervase Markham [:gerv]]

I do not understand this bug report. What is the problem, exactly? 

Also, is it assigned to me just by default, or am I expected to fix it?

Gerv

Comment 2

[Bradley Baetz (:bbaetz)]

Instead of writing out a temporary file, you can embed it in the webpage
directly. This alos means that the browser can show the linnk destination in teh
status bar, and know which areas are not links.

Comment 3

[John Vandenberg]

> Also, is it assigned to me just by default, or am I expected to fix it?

I have a patch so I am guessing I should be re-assigning it to myself

> Instead of writing out a temporary file ...

As data/webdot still needs to retain the generated image, why is there an issue
also retaining the server-side image map.  Then the server-side image map can be
used if the client is not capable of using the client-side image map.

Comment 4

[Gervase Markham [:gerv]]

My templatisation (bug 126793) is next in the queue for munging this file. I
suggest you don't make a patch until that goes in, because I'll probably break
it :-)

Gerv

Comment 5

[John Vandenberg]

Attachment: v1

Includes a client-side image map when locally generating dependency graphs.

Comment 6

[Tom Emerson]

I ended up fixing this in my own Mozilla version and then checked to see if it
had been addressed already... my approach is slightly different, so I've
attached it anyway for perusal.

Without this patch (mine or John's) locally generated graphs are next to
useless: clicking on one of the boxes loads the .map file, which is less than
helpful. If
2.16 is going to ship with local dot files, then the image maps must be client-side.

I don't care whether my version or John's makes it in: but one of them should.

Comment 7

[Tom Emerson]

Attachment: Tree's variant of client-side image maps

My version of this fix, slightly different from John's.

Comment 8

[Bradley Baetz (:bbaetz)]

Without having looked at either of the patches, server side image maps do work
for me on apache.

Comment 9

[Tom Emerson]

Indeed: my Apache configuration does not have server-side image maps enabled,
which happens to be the default in Apache 1.3.x.

Client-side image maps are a better solution in any case, IMHO. 

Comment 10

[Bradley Baetz (:bbaetz)]

Comment on attachment 77761 [details] [diff] [review]
v1

graphviz provides an output mode for client side image maps, you should use
that and then embed the results into teh template.

Comment 11

[Bradley Baetz (:bbaetz)]

Comment on attachment 78284 [details] [diff] [review]
Tree's variant of client-side image maps

Are there browsers which can't handle client side image maps? IE do we want
both available?

Comment 12

[John Vandenberg]

Attachment: v2 : client side image maps only

> graphviz provides an output mode for client side image maps, you should use
> that and then embed the results into teh template.

The graphviz output format I believe you are refering to (ismap) outputs the
following:

default URL label
rectangle (x1,y2) (x2,y1) URL label

webdot internally converts this format into a client-side image map.  

> Are there browsers which can't handle client side image maps? IE do we want
> both available?

Not really.  As a result this patch follows webdot and uses graphviz's ismap
format instead of the imap format.

Comment 13

[Tom Emerson]

With regards to attachment 78322 [details] [diff] [review], the ismap attribute is not needed on the img
attribute when using client-side image maps.

Comment 14

[Tom Emerson]

>graphviz provides an output mode for client side image maps, you should use
>that and then embed the results into teh template.

I don't think you can do this directly: neither the -Tismap nor the -Timap
output can be directly used within a <map> block, AFAICT. My patch converts the
output from the '-Timap' to a series of <area> elements that are then inserted
into the <map> block in the template.

Comment 15

[Bradley Baetz (:bbaetz)]

grr. The graphviz module claimed that imap was serv side and ismap was client
side. Its wrong, though. 

Comment 16

[John Vandenberg]

Thanks for picking that up Tom.  

The ismap format is intended for conversion to client-side image maps,
containing no comments and providing regions with labels.  The coordinates in
the ismap format are also not subject to two different specs (NSCA and W3C).

Comment 17

[John Vandenberg]

Comment on attachment 78322 [details] [diff] [review]
v2 : client side image maps only

Marking needs-work on behalf of Tom.

Comment 18

[John Vandenberg]

Attachment: v4: review fixes + post-templatisation

This removes the (new) dependency on Apache module mod_imap, which many would
prefer to not introduce in this release.  Could we try and get this one in?  It
would simplify the documentation bug 123394 which I am about to work on.

Comment 19

[Tom Emerson]

I really think this needs to get into 2.16: removing the requirement for server-
side image maps is a Good Thing, and being able to correctly use a local 
graphviz installation will make a lot of people really easier.

Comment 20

[Bradley Baetz (:bbaetz)]

Yeah, probably couldn't hurt, but its a P5 for 2.16. It definately gives nicer
results, and it can't break anything from 2.14, since we didn't support client
side maps at all.

the patch loks fine; I'll test it later.

Comment 21

[John Vandenberg]

Attachment: Patch v.5

v.4 only worked in Mozilla.  This patch uses proper syntax to refer to map, and
removes name attribute from generated area when shape=default.

Comment 22

[Bradley Baetz (:bbaetz)]

Comment on attachment 81985 [details] [diff] [review]
Patch v.5

tested, works, r=bbaetz. I'd prefer that we didn't use a templ file, but since
the way to do that breaks on windows, we can live with it for now.

Comment 23

[Dave Miller [:justdave]]

Comment on attachment 81985 [details] [diff] [review]
Patch v.5

r= justdave

Comment 24

[Bradley Baetz (:bbaetz)]

Checked in.