80388 - cvsblame shouldn't require layers for popups

Status: VERIFIED FIXED

(Webtools Graveyard :: Bonsai, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

cvsblame currently uses layers to popup the cvs checking comments when mousing
over a link.  unfortunately this doesn't work in non NetscapeClassic browsers,
significantly cutting down on usefulness.

Comment 1

[Jacob Steenhagen]

Wow, I didn't even realize cvsblame did that!! It's quite slick.  The question
is, what can the layers be replaced with?  My initial thought was a 'title='
attribute in the link, but I don't think tooltips are really up to that task :(

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

A similar conversion was already done to display the comment popup in tinderbox
using DOM stuff.  I suspect that approach might work here as well.

Comment 3

[Dave Miller [:justdave]]

There is a way to do that in IE also, using hidden <SPAN> elements with absolute 
positioning and style sheets. :)

The javascript just needs to check which browser it's running in and run the 
appropriate code.

Comment 4

[Alec Flett]

the html/dom-standard way is with absolutely positioned divs

Comment 5

[Jacob Steenhagen]

Attachment: Playing with DIVS (very rough)

Comment 6

[Jacob Steenhagen]

I just attached some very rough source, but it works in both IE and Mozilla. 
The current problems I see with this approach is that it has a DIV for each link
(essentially, each change of color in cvsblame, but not quite that limited)
which can really add up to a lot of duplicate being transmitted.  It also hides
the DIV again as soon as the mouse leaves the link.  In the current
functionality, it's possible to move over to checkin message and click on any
links that may be there (like bug XXX).

Comment 7

[Alec Flett]

one thing that I think tinderbox does is construct a sort of skeleton DIV, and
then fill in data when the user clicks on a link.. (I could be wrong of course,
it might be creating a DIV for each link.. but the skeleton approach would
definitely work)

Comment 8

[Jacob Steenhagen]

What I'd like to do but haven't been able to figure out how (or if it's even 
possible) is create a <div> for each revision and shove them all at the bottom 
of the page.  Then have the JavaScript that shows the <div> also move it 
relative to either the current mouse position or the position of the link.  I 
discovered that I could move the <div> with a "text.style.left=<number>" 
and "text.style.top=<number>" (where text is the value returned by 
getElementById) but I can't seem to figure out how to get the value to use for 
<number>.

Comment 9

[Dave Miller [:justdave]]

see HTMLElement.offsetLeft, .offsetTop, .offsetWidth, and .offsetHeight in the 
DOM.  Probably .offsetParent, too, if you need to calculate absolute positions.

Comment 10

[Jacob Steenhagen]

Attachment: Code that seems to work

Comment 11

[Jacob Steenhagen]

I've tested this on a recent Mozilla nightly and on IE 5.5 and it seems to work
on both of 'em... so, this will probably be the basic idea I use when I dive
into bonsai (which is the next step :)... unless of course there's an error I
didn't notice.

BTW, this second sample solves both of my issues. It mkaes it so there's one
<div> per revision and when a link is moused-over it moves that div relative to
the link and shows it.  It then keeps displaying it until either another rev
link is moused over or the page is clicked.

Comment 12

[Jacob Steenhagen]

Attachment: Use DOM for Mozilla and IE 5+

Comment 13

[Jacob Steenhagen]

OK, I've tested this patch on my local install of bonsai w/Mozilla, but I
haven't had a chance to try it with IE yet (the sample page worked, so I see no
reason why the patch wouldn't).  Basically what this does is add another level
of detection to see if layers should be used or the standard DOM (or nothing at
all).  Then each place "if $use_layers" was I added code support for "if
$use_dom".  I reused code wherever possible so the popup actually looks almost
identical in Mozilla as it does in Netscape 4.

The one thing I can think of that may seem a little stange is that everything
for the .log_msg class is in a <style> definition with the exception of
"display: none".  For some reason if I put that in the style, then the
JavaScript couldn't cause it to be displayed (in both IE and Moz, so that must
be by design).  I also used a style definition for the body stuff and link stuff.

Comment 14

[Dave Miller [:justdave]]

> everything for the .log_msg class is in a <style> definition with
> the exception of "display: none".  For some reason if I put that in
> the style, then the JavaScript couldn't cause it to be displayed

try "visibility: hidden".

Comment 15

[Jacob Steenhagen]

I get the same results with "visibility: hidden"

Comment 16

[Jacob Steenhagen]

Attachment: Patch v2

Comment 17

[Jacob Steenhagen]

I discovered a couple minor problems and a major problem with the first patch. 
The major one was that it didn't work w/IE 5+.  The reasoning for this was
two-fold.  The first was that I did the detection backwards so it thought IE 5
was just another 4- browser.

The second reason is where it got ugly.  It seems that .offsetTop returns how
far down an element is from its parent, not the top of the page.  In Mozilla
this wasn't a bug deal because the offsetParent for the A tags was in fact the
BODY.  For some reason, IE made the offsetParent for these same A tags the table
cell, so most of the Popups were showing up at the top of the page (because the
A was 0 pixels from the top of the table cell).  I got around this by adding a
loop that keeps adding the offsetTop value until it reaches the BODY.

So, Patch v2 has been tested in both Mozilla and IE 5.5 (I've not yet tested 5.0).

Comment 18

[Jacob Steenhagen]

r=baloo in IRC

Checked In.

Comment 19

[timeless]

vrfy fixed
1.25 jake%acutex.net May 30 2001