73970 - Tooltips disappear when at bottom of screen

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Colin Phillips]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.16 i686; en-US; 0.8.1) Gecko/20010326
BuildID: 2001032614

When the mouse cursor is too close to the bottom of the screen, the tooltip pops
up and immediately disappears.  The behaviour should instead be that if the
mouse if too close to bottom of the screen, the tooltips should appear above the
mouse cursor, not always below or beside.

Reproducible: Always

Comment 1

[:deanis74]

I see this on Windows as well...

1. Went to the build comments page at mozillazine
(http://www.mozillazine.org/build_comments/) and moused over a tooltip.
2. Showed fine.
3. Moved my browser window to line up the bottom with the bottom of my screen.
4. Scrolled so the last full line showing was the March 21 comment line
mentioning bug 71962.
5. Also left about half of the next line visible, so that I could see the link
to bug 63103.
6. Moused over links on the bottom three lines with the following results:
   72696: Tooltip displayed in "normal" position, below mouse.
   71962: Tooltip displayed below mouse, but higher than "normal", which
          is good otherwise it would have been partially off-screen.
   63103: No tooltip displays.  Well, there's a little flicker of the cursor
          then nothing, so I wonder if the tooltip is being displayed briefly.

Given the fact that the tooltip does get repositioned in some cases, like in the
71962 link, I think the repositioning code is just a little "off".  Guessing
this is pink.

Should the platform/os be all/all based on my observations?  I'll leave that up
to someone else to determine if it's in XP code or two different situations that
are just really similar.

Comment 2

[:deanis74]

cc: me.  Maybe I can do something about this pre-future.  Mike, is the tooltip
positioning done in GetFrameForView (or whatever) ?

Comment 3

[Mike Pinkerton (not reading bugmail)]

SyncViewWithFrame

Comment 4

[:deanis74]

This sounds like bug 48134.  One of these should probably be duped.

Comment 5

[Colin Phillips]

What is the status of this?

Comment 6

[Colin Phillips]

This works better in 0.9.2, but still not very well.  If you mouse over the very
bottom of the item (at the bottom of the screen), it doesn't actually pop
anything up, but it appears to think it has, and for some reason, the item
you're hovering over loses its "hover" state.

Comment 7

[h.shum]

In Windows ME:

When the "Auto-hide" option is turned on for the Windows Start Menu Bar (i.e.
the Windows Start Button & System Tray would pop up when the mouse is dragged to
the bottom of the screen) AND the Mozilla Windows is maximised, the tool tips of
Mozilla's Status Bar never shows.

Comment 8

[Colin Phillips]

Hyatt:

Based on your recent changes to XUL 1.0, this is much better, but still not
entirely correct.  

Using 2001080800 For linux I get the following problem.

Steps to reproduce:
1) move the window into the middle of the screen so that the bottom of the
mozilla window is at least 100 px from the bottom of the screen
2) mouse over the address book button on the status bar 
--> note the popup appears correctly
3) move the window down the bottom, so that the mozilla window is flush with the
very bottom of the screen
4) mouse over the address book button again
--> note the popup does not appear correctly. (it seems like it is trying to
appear, but then disappears before it actually shows on screen.  note also that
the mouse appears to flash to indicate this)
--> also note that the security popup DOES appear correctly

Comment 9

[John Morrison]

*** Bug 94841 has been marked as a duplicate of this bug. ***

Comment 10

[:deanis74]

I want to look at the tooltip code sometime soon because I don't like how 
they're re-positioned if they don't fit horizontally.  I'll try and look at 
this at the same time.

Comment 11

[Mike Pinkerton (not reading bugmail)]

tooltips -> hewitt

Comment 12

[Mike Potter]

OEone would really love to have this bug looked at before sometime in the
future. We've found that tooltips are really helpful to the end user who doesn't
know what the buttons do, and this is blocking the tooltips from appearing, thus
making the product more difficult to use. Its probably not a big deal for those
who use Mozilla (or OEone Homebase) everyday, but to new users this could be a
big issue.
I talked to pink on IRC, and he said that the tooltip was being rendered under
the mouse, which then causes the tooltip to disappear right away.
Bumping up severity from minor to normal. I won't go higher than that, but I do
think this is a problem for end users.

Comment 13

[:deanis74]

What about something as simple as not having tooltips dismissing on mousemove?

Comment 14

[Fabian Schach]

Marking All/All

Note that this issue does not occur with tooltips on the lower right side
(online/offline and security icons), only with the toolbar on the left.

Comment 15

[:deanis74]

It doesn't happen with tooltips on the lower right side because if they don't
fit they're moved to the left side of the cursor.  (Aside: This isn't the proper
behavior, they should instead be shifted left until they display fully.)

Comment 16

[m_mozilla]

WFM Mac OS X build 2002020809

maybe this isn't all/all?

I tried putting just a couple pixels of the (lower right) security lock icon
visible in the lower left corner of my screen.

I tried putting just a couple pixels of the (lower left) browser quick access
icon visible in the lower right corner of my screen.

I tried putting both icons just barely peeking above the lower edge of my
monitor, and tried a normal bugzilla bugnumber link.

In all cases, the tooltip rendered just fine.

Has anyone else reproduced this on Mac OS X? Maybe we have an immunity that
might provide a clue toward a solution?

-matt

Comment 17

[:deanis74]

Any objections to me taking this?  What I'd like to try is to re-work the
positioning of tooltips to address three issues:

1. Don't switch to the right side of the pointer if there's not enough room on
the left.  Instead, shift it to the left until it will display fully on-screen.
2. Make sure it's still completely on the screen at the left side.  If not,
position at the left of the screen and trim the width to the screen width.
3. Ensure that the tooltip doesn't appear under the pointer.  Instead of simply
moving the tooltip up until it's on-screen, move it up until it's both
completely on-screen and not under the pointer.

Addressing issue #3 specifically should fix this bug.

Comment 18

[John Morrison]

mikep: did you really mean to set this 'Future'? [Don't think so, given the 
text of your message :-]

unsetting milestone. Dean, if you want to look at this, go for it.

Comment 19

[Mike Potter]

It was targeted future, and I don't like changing stuff like that, unless I'm
the one who is actually going to fix the bug (which I can't).
I suggest that Dean set the Milestone to whatever he wants. There's a big OEone
thank you to him when this gets done. :)

Comment 20

[Peter Bojanic]

Nominating for mozilla1.0. This bug results in a serious Penzilla usability
issue because the tooltips for the launch pad buttons at the bottom of the
screen usually don't show up properly.

Comment 21

[mental]

*** Bug 144439 has been marked as a duplicate of this bug. ***

Comment 22

[Itamar]

Hi,
Doing a little investigation myself I found the following problem - for Win32 (maybe its the same for 
linux but I don't really know)

When a tooltip is created a mouse timer is created with it and the handle of the window that is currently 
under the mouse pointer is saved - the timer checks the mouse postion each time tick.
Each timer tick the handle of the window that the is currently under the mouse pointer is comapred to the 
window handle that was saved. in most cases the handle is the same and nothing happens so the tooltip ok.
But tooltip at the bottom of the screen are pushed upwards and result in being under the mouse pointer...
What happens is that the handle that is saved in the timer is comapred to the handle of the tooltip and 
not to the handle of the window it was over just before the tooltip was created.
The timer decides then that the mouse has probably moved out of the window it was over and fires a 
MOUSE_EXIT event - which closes the tooltip.
What probably needs to be done is to make sure that we don't create the tooltips under the mouse pointer -
 while this souds easy, I coudn't find the code that changes the X,Y cord's of the tooltip or in any case 
a place that figures out where should tooltips that exceed the screen should be repositioned.
If anyone can point me to this place I'll be more than happy to continue my investigation... :-)

Comment 23

[:deanis74]

Itamar: Nice find.  The hacky way to fix it would be to not fire the
NS_MOUSE_EXIT if the window handle is the tooltip's window handle.  The better
way, as you mention, would be to fix the positioning.  I think addressing my
comments in point 17 would handle this.

For the positioning, start with nsMenuPopupFrame::SyncViewWithFrame.

http://lxr.mozilla.org/mozilla/source/layout/xul/base/src/nsMenuPopupFrame.cpp#849

Comment 24

[Itamar]

Attachment: path 1.0

This patch works but needs work - what it does is if it detects that the popup
is going to be cut of the screen and if the popup is a tooltip it recalculates
everything to postion the tooltip just above the mouse pointer. 
It works but seems to much work and not very generic.

Any ideas about how to improve it? :-/

Comment 25

[:deanis74]

Cool.  Any chance of addressing points 1 and 2 from comment 17 at the same time?

Comment 26

[Itamar]

About point #1:
I don't see any code doing that except for the MovePopupToOtherSideOfParent
function - but when I think of it when do u get a situation where the tooltip
doesn't have room on the left?
Tooltip are aligned to the left side of the pointer and since the pointer can
never leave the sceen they always have room the left. 
http://lxr.mozilla.org/mozilla/source/layout/xul/base/src/nsMenuPopupFrame.cpp#1232
deals with the other case when there is no root to the right and shift it to the
left - this can be corrected a bit.
About Point #2:
I guess it could be done with not too much trouble.
I'll fix it in the next patch but I really need to make the first patch nicer
first. The special case for the tooltips is probably needed but doing so many
calcs for just repostioning the tooltip over the pointer seems a bit too much.
Anyway can you explain point 1 again and I'll fix them for the next patch.

Comment 27

[:deanis74]

Sorry, I think that was a typo in #1.  It should be not enough room on the
_right_, not left.  The behavior in #1 happens because the code is the same for
popup menus and tooltips.  For popup menus it makes sense to change from
left-aligned to right-aligned relative to the pointer, but for tooltips it makes
more sense to shift to the left until it can be displayed.

It's been a little while since I thought about these, and I don't think #2 is
valid anymore.  We used to not crop long tooltips, so they could be as wide as
the screen.  If they flipped from left-aligned to right-aligned, the long
tooltips would start off the left side of the screen.

Comment 28

[Itamar]

ok - I'll post a new patch today addressing Point #1 as well.

Comment 29

[:deanis74]

Comment on attachment 93606 [details] [diff] [review]
path 1.0

>+        AdjustClientXYForNestedDocuments ( xulDoc, presShell, aXPos, aYPos, &newXPos, &newYPos );

AdjustClientXYForNestedDocuments() is already called in SyncViewWithFrame(), at
line 964.  Why call it again?

http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsMenuPopupFrame.cp
p#964

Comment 30

[Itamar]

Yep Thats what I thought too.
I need to call it again to get the newYPos since ypos it too messed up at this
stage and I don't know where is the mouse pointer any more - may I should take
out newYPos of the first block for use in later time.

Comment 31

[Itamar]

Attachment: patch 1.1

This patch addresses points number 1 and 3 of comment 17.
My consern is with comment just above the fix I made for point 1
"    // as a result of moving the popup, it might end up under the mouse. This
      // would be bad as the subsequent mouse_up would trigger whatever
      // unsuspecting item happens to be at that position. To get around this,
make
      // move it so the right edge is where the mouse is, as we're guaranteed
      // that the mouse is on the screen!"
It seems the flip postioning was made on prpose, but 2 years ago so maybe the
comment is absolete.

The fix for point 3 works but still i'm not sure using the newYPos that was
calculated early in the function is good.
Anyway I tested the patch as much as I can and it seems to cause no
regressions.
Still i'm open to any ideas about how to improve it.

Comment 32

[:deanis74]

Itamar: I'm pretty sure that comment is mostly for popup menus, as the actions
for menu items fire on mouseup.

Comment 33

[Itamar]

I see - this means that with this patch it will make the context menu popup
under the mouse pointer.
So maybe I should change the patch to be somthing like:

       if (tag.get() == nsXULAtoms::tooltip) {
         xpos -= (screenViewLocX + mRect.width) - screenRightTwips;
       }
       else
         xpos -= mRect.width;

instead of just replacing the 
  xpos -= mRect.width;
with 
  xpos -= (screenViewLocX + mRect.width) - screenRightTwips;

Comment 34

[:deanis74]

Yes, definitely.  All of these changes should be for tooltips only.

Comment 35

[:deanis74]

You'd also need to change or add to the comment, which could make things more
confusing.  Maybe it would be better to check first if it's a tooltip or not,
and then do the positioning.

Also, you don't need tag.get(), just use tag.

Comment 36

[Itamar]

Attachment: patch 1.2

This one has all the changes discussed in the last comments.
Any other changes - or is this good enough for review?

Comment 37

[:deanis74]

Comment on attachment 94191 [details] [diff] [review]
patch 1.2

Looks good to me.  I applied the patch and tooltips behave more like "normal"
tooltips.  The patch doesn't apply cleanly, but I was able to add the missed
chunk in by hand.

- remove the {} around your one-line if statements
- use spaces to indent, not tabs

Fix that and r=me.

Comment 38

[Itamar]

Attachment: patch 1.3

Evil tabs.
Hope I got them all removed.
Other then tabs and the {} why didn't the patch go cleanly?

Comment 39

[:deanis74]

patching file `src/nsMenuPopupFrame.cpp'
Hunk #1 succeeded at 968 (offset 15 lines).
Hunk #3 FAILED at 1224.
1 out of 3 hunks FAILED -- saving rejects to src/nsMenuPopupFrame.cpp.rej

From your patch, "diff -u -2 -r1.170 nsMenuPopupFrame.cpp".  This file is up to
revision 1.177 now.  Remove the "-r1.170" parameter and it will diff against the
tip version.  Also remove "-2" so the patch contains more context lines.

Comment 40

[Itamar]

Attachment: updated patch 1.3

ok this updaed for 1.177
And I've paid attention not to setover the recent change for bug 120226
Keeping my fingers crossed. :-)

Comment 41

[:deanis74]

I think you've written over the fix for bug 120226.  That patch used margin.top
and margin.bottom (see bug 101677), but you simply use (int)p2t.  I don't know
if that's right.  cc: jerry tan and bryner for comments.

Also test to make sure that any patch doesn't break what 120226 fixes.

> //so the mouse won't think it moved - see bug 73970

Don't need to mention the bug number.

Comment 42

[Itamar]

Hi,
I havn't compiled mozilla for a long time now - and wasted a lot of time trying
to compile it with the new make configuration.
Well I can't say I made a lot of progress so far.
Any way I have to go for a while and won't be able to continue updating this
patch or answer emails untill Sept 13th.
So Dean, If you could pick up from where I left It would be great - if not then
it will just have to wait a bit. Sorry.

Comment 43

[Itamar]

Hi,
I came back a few days ago - early then I expected.
Downloaded a fresh source snapshot and managed to compile it.
To my complete surprise the bug is gone – I still haven’t investigated when was
it fixed but it seems to be working for me.
Point 1 in comment 17 is not fixed and I can modify the patch to do just that –
if other people confirm that the bug is fixed in the latest builds.
So what do you say…

Comment 44

[:deanis74]

Wow, that's really weird!  I never noticed it before, but point 3 of comment 17
looks to be addressed in 2002082908.  Not sure when it started working properly.
 I'd be fine if you addressed just point 1 (and thus point 2).

Comment 45

[Itamar]

Attachment: New patch to cover point 1 in comment 17

Ok found out who fixed it - 
Its the fix for bug 120226 see the patch.
Actually it much simpler than mine and works - so cool...

This patch addresses point 1 in comment 17 so tooltips are moved to the left
and not flipped like menus.

Comment 46

[:deanis74]

Comment on attachment 97648 [details] [diff] [review]
New patch to cover point 1 in comment 17

Do we need to worry about point 2 in comment 17?  I guess probably not.  The
current code just chops off mRect.width, and we'll always be chopping off less
than that if we're a tooltip.  I think I wrote that back when tooltips weren't
cropped, so they could actually be wider than the screen.

Looks good.  Nice and simple now that the other bug is fixed.  r=me

Note to potential super-reviewers: I think this is a good patch to get in. 
It's low-impact, as it only affects tooltips, but it's another step towards
making our tooltips appear more like "standard" tooltips (whatever those may
be).

Comment 47

[Brian Ryner (not reading)]

Comment on attachment 97648 [details] [diff] [review]
New patch to cover point 1 in comment 17

sr=bryner

Comment 48

[:deanis74]

Is this comment still valid?  It's right above the code that the patch adds.  I
think it's old, since the tooltip will never be under the mouse vertically anymore.

      // as a result of moving the popup, it might end up under the mouse. This
      // would be bad as the subsequent mouse_up would trigger whatever
      // unsuspecting item happens to be at that position. To get around this, make
      // move it so the right edge is where the mouse is, as we're guaranteed
      // that the mouse is on the screen!

Comment 49

[:deanis74]

> I think it's old, since the tooltip will never be under the mouse vertically
> anymore.

Not because of this fix, but from some fix that was checked in a while back, not
sure when.

Comment 50

[Itamar]

Dean I think the cooment is still valid for the line:
xpos -= mRect.width;

"...move it so the right edge is where the mouse is..."
This what happens in menu popups.
For tooltips there is a different handling.

You are right in a sense that it should never happen that a popup will be under
the mouse, and in that stage of the code moveing the tooltip left or right will
not cause the mouse point to be over the popup but the mouse cursor itself might
very well be. Just r-click near right edge of the screen and see that if the
popup menu was a little more to the right the mouse cursor would have covered it
a bit.
I thnk you can check the patch and leave this comment as is.

Comment 51

[:deanis74]

I don't want to leave it in if I check in this patch, because the code below it,
being the new code, does not reflect what the comment says.

I think the comment is out of date anyway, though, because I recall that
tooltips used to only shift up when they were off the screen, not flip to being
completely above the pointer.  I think, really, that this comment should have
been removed with the fix for bug 120226.  cc: jerry tan to confirm

bryner, can I remove this comment with my check-in?

Comment 52

[Itamar]

Dean, I don't think you'll get an answer any time soon.
Just remove it and check this in...

Comment 53

[:deanis74]

Sorry, I can't.  I need bryner's - or another super-reviewer's (Blake...?) -
approval before I can make that change.

Comment 54

[Shervin]

I tried this with the source code of mozilla 1.0.1 and Brian's patch (
attachment 97648 [details] [diff] [review] ), I still see a problem for buttons that are close to the
bottom of the screen. It's very strange, I'll try my best to explain it:

I have a button 3 pixels from the bottom edge, mozilla maximized at a 1024x768
resolution with no window chrome (no titlebar, no edges, no status bar...). When
I hover on the button (button goes into hover mode as defined in css) and stop
the mouse close to the bottom of the button, the button loses the hover mode and
no tooltip is shown (Yes, I'm still inside the button). When I do the same thing
but stop the mouse somewhere in the middle of the button, as opposed to close to
the bottom edge, hover state is preserved and tooltip is shown.
When I move mozilla away from the bottom of the screen, the problem goes away.

Comment 55

[Itamar]

Shervin, read comment 45 to know why the last patch doesn't fix the problem on 1.0.1

If you want a patch that will you should try the fix for bug 120226 or the
updated patch 1.3 posted here.
Hope this helps...

Comment 56

[:deanis74]

Checked in, sans comment.