wrong (last or empty) tooltip text displayed for dropdown list menu items

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
XUL
RESOLVED FIXED
15 years ago
7 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: neil@parkwaycc.co.uk)

Tracking

({polish, testcase})

Trunk
mozilla1.9alpha1
polish, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 -
blocking1.8.0.2 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(10 attachments, 6 obsolete attachments)

128.98 KB, image/jpeg
Details
1.12 KB, patch
Dean Tessman
: review+
Brian Ryner (not reading)
: superreview-
Details | Diff | Splinter Review
11.91 KB, patch
neil@parkwaycc.co.uk
: review+
Brian Ryner (not reading)
: superreview-
Details | Diff | Splinter Review
680 bytes, patch
Brian Ryner (not reading)
: superreview-
Details | Diff | Splinter Review
1.04 KB, application/vnd.mozilla.xul+xml
Details
11.46 KB, patch
Louie Zhao
: review+
Henry Jia
: superreview+
Details | Diff | Splinter Review
1.36 KB, patch
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
15.98 KB, patch
jst
: review+
Details | Diff | Splinter Review
747 bytes, application/vnd.mozilla.xul+xml
Details
1.65 KB, patch
jst
: review+
Details | Diff | Splinter Review
tooltip for delete button showing up when I'm using the file button menulist

I was trying to file a message, and the delete button tooltip kept showing up.

in the attached screen shot, my mouse was above the 1800contacts item (which is 
selected).
Created attachment 85290 [details]
screen shot of the tooltip up when filing
Created attachment 85292 [details]
screen shot of the tooltip up when filing
Attachment #85290 - Attachment is obsolete: true

Comment 3

15 years ago
In fact, it's the last tooltip you had seen. IMO, menuitem is not supposed to 
have a tooltip.

Comment 4

15 years ago
Created attachment 91047 [details] [diff] [review]
looking up "tooltiptext" attribute form mTargetNode instead of mSourceNode

That guarantee we'll get the tooltiptext from menuitem itself rather than its
parent - toolbar button.

Comment 5

15 years ago
it's a general bug, not for mailnews only. taking...
Assignee: sspitzer → kyle.yuan
Component: Mail Window Front End → XP Toolkit/Widgets
Product: MailNews → Browser

Comment 6

15 years ago
*** Bug 139381 has been marked as a duplicate of this bug. ***

Comment 7

15 years ago
Created attachment 91054 [details] [diff] [review]
add checking for null pointer


seeking r=
Attachment #91047 - Attachment is obsolete: true

Updated

15 years ago
Keywords: review

Comment 8

15 years ago
*** Bug 158280 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Keywords: mozilla1.2, polish

Updated

15 years ago
Alias: tooltips

Updated

15 years ago
Keywords: nsbeta1

Comment 9

15 years ago
*** Bug 151270 has been marked as a duplicate of this bug. ***

Updated

15 years ago
Depends on: 164639

Updated

15 years ago
No longer depends on: 164639

Updated

15 years ago
Depends on: 20016
*** Bug 146200 has been marked as a duplicate of this bug. ***
cc'ing hyatt@netscape.com -- oops, he is listed as module owner, but he is not
in bugzilla... is hyatt@apple.com the same person? I think review belongs to
him.  I applied this patch and it seems to work cleanly, needs R=

Comment 12

15 years ago
Benjamin, thanks for the verifying. hyatt is working for Apple now, so he
changed his email address.

Comment 13

15 years ago
Kyle, will this fix bugs like bug 133788 as well?

Comment 14

15 years ago
yes, it does. Dean, could you r=?
Blocks: 133788

Comment 15

15 years ago
oops, it doesn't :( I'll try to make another patch for bug 133788.
No longer blocks: 133788

Updated

15 years ago
QA Contact: olgam → laurel

Comment 16

15 years ago
*** Bug 186009 has been marked as a duplicate of this bug. ***

Comment 17

15 years ago
*** Bug 184710 has been marked as a duplicate of this bug. ***

Comment 18

14 years ago
*** Bug 181617 has been marked as a duplicate of this bug. ***

Comment 19

14 years ago
*** Bug 161322 has been marked as a duplicate of this bug. ***
WFM with 5/16 build.
QA Contact: laurel → esther

Comment 21

14 years ago
I can still reproduce with the 2003052004 build on Windows 98 with a new profile.

Comment 22

14 years ago
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-

Comment 23

14 years ago
*** Bug 213244 has been marked as a duplicate of this bug. ***

Comment 24

14 years ago
*** Bug 201162 has been marked as a duplicate of this bug. ***

Comment 25

14 years ago
*** Bug 220738 has been marked as a duplicate of this bug. ***

Comment 26

14 years ago
*** Bug 190199 has been marked as a duplicate of this bug. ***

Updated

14 years ago
Blocks: 178215

Comment 27

14 years ago
*** Bug 228273 has been marked as a duplicate of this bug. ***

Comment 28

14 years ago
The patch still applies and works fine for me.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031213
Firebird/0.7+ (Steffen)

The effect is that tooltips only appear on menu-buttons, but not on the dropdown
menuitems. But you can still add a tooltiptext to each menuitem if you want.
Other tooltips on the navigation toolbar, the bookmarks toolbar and the
bookmarks sidebar are not affected.

Adjusting the summary to reflect the scope of this bug and its 13 dupes.
Summary: tooltip for delete button showing up when I'm using the file button menulist → tooltips of menu-buttons appear on the dropdown menuitems

Comment 29

14 years ago
Comment on attachment 91054 [details] [diff] [review]
add checking for null pointer

Someone should've requested review 16 months ago.
Attachment #91054 - Flags: review?(jag)

Comment 30

14 years ago
Created attachment 137368 [details]
testcase

I've made a testcase for bug 228273. This is a modified version.

This shows two menu-buttons. The left one has a tooltip only at the button. The
right one has a tooltip on the menuitems as well.

Without the patch:
1. Hover over the left button. The tooltip "Print this page" appears.
2. Click the dropmarker. Hover over one of the menuitems. The tooltip appears
as well. That's the bug.
3. The right button is working properly: tooltips on the button and the
menuitems.

With the patch applied:
1. Hover over the left button. The tooltip "Print this page" appears.
2. Click the dropmarker. Hover over one of the menuitems. There's no tooltip
because there shouldn't be one.
3. Hover over the right button. The tooltip "Print this page" appears.
4. Click the dropmarker. Hover over one of the menuitems. You'll notice a
tooltip on each menuitem. So we can still have specific tooltips on menuitems
if we really want.

Updated

14 years ago
Keywords: testcase

Comment 31

14 years ago
Comment on attachment 91054 [details] [diff] [review]
add checking for null pointer

I'll bite, sure.  r=me.  Jag, care to sr=?
Attachment #91054 - Flags: superreview?(jag)
Attachment #91054 - Flags: review?(jag)
Attachment #91054 - Flags: review+

Comment 32

14 years ago
Comment on attachment 91054 [details] [diff] [review]
add checking for null pointer

Looking at this, I'll have to understand how this code works before I can sr,
you might want to ping bryner, I could maybe get to it later this week.

Comment 33

14 years ago
Comment on attachment 91054 [details] [diff] [review]
add checking for null pointer

Brian, Jag said we might want to ping you on this.
Attachment #91054 - Flags: superreview?(jag) → superreview?(bryner)

Comment 34

14 years ago
Created attachment 137583 [details] [diff] [review]
add buttontooltiptext and use it in most places

Updated

14 years ago
Attachment #137583 - Flags: superreview?(dbaron)
Attachment #137583 - Flags: review?(neil.parkwaycc.co.uk)

Updated

14 years ago
Assignee: kyle.yuan → timeless
Component: XP Toolkit/Widgets → XP Toolkit/Widgets: XUL

Comment 35

14 years ago
fwiw my patch can be moved to bug 71329 if people really care...

Comment 36

14 years ago
Comment on attachment 137583 [details] [diff] [review]
add buttontooltiptext and use it in most places

I know this probably isn't the place to get into this, but |buttontooltiptext|?
 That seems backwards to me.  It makes more sense to use the standard
|tooltiptext| for buttons and add something like |dropdowntooltiptext|, as
mentioned a couple of times in bug 71329.  We currently use |buttontooltiptext|
a total of one place that I could find, navWidgetBindings.xml.	But, as usual,
that's probably just me.

Or wait, theme authors probably use |buttontooltiptext|, right?
(Assignee)

Comment 37

14 years ago
Comment on attachment 137583 [details] [diff] [review]
add buttontooltiptext and use it in most places

r=me if you patch modern's globalBindings.xml too.
Attachment #137583 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Comment 38

14 years ago
I had a look at how tooltiptext is implemented, and I think attachment 91047 [details] [diff] [review] is
a worse kludge than attachment 137583 [details] [diff] [review], although my preference currently leans to
a separate per-window tooltiptext listener.

Comment 39

14 years ago
Comment on attachment 137583 [details] [diff] [review]
add buttontooltiptext and use it in most places

I agree with Dean, we should be backward compatible. tooltiptext should be for
the button and lets have something else for the dropdown.

I think "tooltiptextdropdown" is  more appropriate, because indented xul files
will look better if one specify tooltiptext and tooltiptextdropdown, and
because in dialogs, we already have ondialogaccept, ondialogcancel,
ondialogextra1 etc.
Let's be consistent, here.

In the new toolkit, I already have added buttonlabelaccept,
buttonlaccesskeycancel etc.

Comment 40

14 years ago
another argument against "tooltiptextbutton" for the button is that
"tooltiptext" and "tooltiptextdropdown" could be used in menulist also,
otherwise, we would need to define and memorize another attribute
"tooltiptextmenulist".
Attachment #137583 - Flags: superreview?(dbaron) → superreview?(bryner)
Comment on attachment 137583 [details] [diff] [review]
add buttontooltiptext and use it in most places

I agree with Pierre on this -- I don't like needing to change tooltiptext to
buttontooltiptext.  The attribute is on the button element, therefore it should
already be assumed that it should apply to the button itself, not the dropdown.
Attachment #137583 - Flags: superreview?(bryner) → superreview-
Comment on attachment 91054 [details] [diff] [review]
add checking for null pointer

This doesn't seem quite right to me.  The source and target nodes should be
able to differ and the tooltip text should always come from the source node --
this is needed to allow tooltips from parent elements to apply when the mouse
is over a child element.

What it seems like we need here is for the menupopup element to effectively
"block" the parent tooltip from applying to itself and its children.  Maybe we
could just give the menupopup a |tooltiptext=""|?  Or introduce another
attribute that could be used on menupopups inside toolbarbuttons.
Attachment #91054 - Flags: superreview?(bryner) → superreview-

Comment 43

14 years ago
> Maybe we could just give the menupopup a |tooltiptext=""|?
Adding tooltiptext="" to menuitems doesn't work. See attachment 137303 [details] (attached
to bug 228273). That testcase shows two menu-buttons. The right one has
tooltiptext="" on the menuitems.
1. Hover over the right button. The tooltip "Print this page" appears.
2. Click the dropmarker. Hover over one of the menuitems. An empty box appears
as tooltip on the menuitems.
(Assignee)

Comment 44

14 years ago
Created attachment 138949 [details] [diff] [review]
Block menupopup tooltips

This uses event.preventBubble; would you prefer to use preventDefault?
(in which case the tooltip listener would have to be revised)
(Assignee)

Updated

14 years ago
Attachment #138949 - Flags: superreview?(bryner)

Comment 45

14 years ago
*** Bug 231031 has been marked as a duplicate of this bug. ***

Comment 46

14 years ago
Comment on attachment 138949 [details] [diff] [review]
Block menupopup tooltips

mousemove events are used by the tooltip listeners, but parents of the content
node hovered on should still get mousemove event notifications.

Comment 47

14 years ago
Created attachment 139355 [details]
tooltip multiple test case

In order to fix that bug and more generally the tooltip code, we need some
specs.
Therefore, I attach a multiple testcase. The vbox contains 4 elements that have
a tooltiptext:
A: an empty yellow hbox
B: a orange hbox that contains a red spacer
C: a toolbarbutton
D: a menulist

B,C,D contain "One" and "Two" children. Only "Two"s have a tooltip text
attribute. The red spacer has no tooltiptext.

In addition to the bug reported here with menupopups, it evidences two bugs:
1) (re)load the testcase, hover on the yellow box wait until the tooltip
appears, then hover downwards on the orange box: the tooltip text is not
updated.
2) hover on the two label in the orange box then rightwards, hover on the red
box: tooltip fails to launch until it is retriggered.

Comment 48

14 years ago
The first bug shows that the current code doesn't cascade correctly. The
expected behavior is either :
- orange hbox tooltip
- no tooltip
whether tooltips are supposed to cascade or not.

I found no specifications on w3.org for the title attribute. 
I guess we should cascade and, but defer to Hixie.
Comment on attachment 138949 [details] [diff] [review]
Block menupopup tooltips

heh, sorry to keep minusing these patches.

I talked with Pierre about this and we decided that we do not want to block the
parent from getting the mouse move, only prevent it from displaying its
tooltip.  This may involve adding some extra information into the event.
Attachment #138949 - Flags: superreview?(bryner) → superreview-
yeah, title should "inherit".

Comment 51

14 years ago
*** Bug 231490 has been marked as a duplicate of this bug. ***

Comment 52

14 years ago
*** Bug 230694 has been marked as a duplicate of this bug. ***

Comment 53

14 years ago
*** Bug 232665 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 54

14 years ago
*** Bug 233286 has been marked as a duplicate of this bug. ***

Comment 55

14 years ago
Does the fix for Bug 233286 that was checked in today have any effect on this?

Comment 56

14 years ago
Nope.

Comment 57

14 years ago
Alright, on mozilla build 2004021608 the problem is partially solved.
On the browser it's fine.
On mail/news, empty tooltips and incorrect tooltips still appear on the smileys
drop-down menu, but the empty lines don't show up anymore...
keep going!

Comment 58

13 years ago
*** Bug 238424 has been marked as a duplicate of this bug. ***

Comment 59

13 years ago
*** Bug 246574 has been marked as a duplicate of this bug. ***

Comment 60

13 years ago
*** Bug 251063 has been marked as a duplicate of this bug. ***
this is on firefox 0.9 branch

flagging blocking-aviary1.0+ 
Flags: blocking-aviary1.0+

Comment 62

13 years ago
Correct me if I'm wrong Tracy, but you don't have permission to set blocking+
flags.  I've change your blocking+ to a blocking request.
Flags: blocking-aviary1.0+ → blocking-aviary1.0?

Comment 63

13 years ago
Not blocking 1.0, but a patch would be cool.
Flags: blocking-aviary1.0? → blocking-aviary1.0-

Comment 64

13 years ago
*** Bug 255475 has been marked as a duplicate of this bug. ***

Comment 65

13 years ago
Maybe we can make a vote;)to decide the solution as follow:
One is prohibit the display to avoid the empty tip or the wrong tip.
Another is define every tips to avoid the empty and the wrong.

Comment 66

13 years ago
Created attachment 159380 [details] [diff] [review]
Define very tips in the UI

If pointers are dangerous to change,how about define every tips in UI?

Updated

13 years ago
Attachment #159380 - Flags: review?(pete.zha)

Comment 67

13 years ago
Comment on attachment 159380 [details] [diff] [review]
Define very tips in the UI

r=pete.
I think it's even better than display nothing
Attachment #159380 - Flags: review?(pete.zha) → review+

Updated

13 years ago
Attachment #159380 - Flags: superreview?(bryner)

Updated

13 years ago
Attachment #159380 - Flags: superreview?(bryner) → superreview?(neil.parkwaycc.co.uk)

Updated

13 years ago
Assignee: timeless → sandy.sun
QA Contact: esther → timeless
(Assignee)

Comment 68

13 years ago
Comment on attachment 159380 [details] [diff] [review]
Define very tips in the UI

Since this actually affects mail compose I suggest you look for a suitable
owner.
Attachment #159380 - Flags: superreview?(neil.parkwaycc.co.uk)

Updated

13 years ago
Attachment #159380 - Flags: superreview?(daniel)

Comment 69

13 years ago
>+<!ENTITY smiley5Cmd.tooltip "Insert a langhing face">
>+<!ENTITY smiley6Cmd.tooltip "Insert a embarrassed face">
>+<!ENTITY smiley7Cmd.tooltip "Insert a undecided face">
>+<!ENTITY smiley8Cmd.tooltip "Insert a surprise face">
>+<!ENTITY smiley10Cmd.tooltip "Insert a yell face">
>+<!ENTITY smiley14Cmd.tooltip "Insert a innocent face">
>+<!ENTITY smiley15Cmd.tooltip "Insert a cry face">

I believe these should be:

+<!ENTITY smiley5Cmd.tooltip "Insert a laughing face">
+<!ENTITY smiley6Cmd.tooltip "Insert an embarrassed face">
+<!ENTITY smiley7Cmd.tooltip "Insert an undecided face">
+<!ENTITY smiley8Cmd.tooltip "Insert a surprised face">
+<!ENTITY smiley10Cmd.tooltip "Insert a yelling face">
+<!ENTITY smiley14Cmd.tooltip "Insert an innocent face">
+<!ENTITY smiley15Cmd.tooltip "Insert a crying face">

Comment 70

13 years ago
Many thanx.

(In reply to comment #69)
> >+<!ENTITY smiley5Cmd.tooltip "Insert a langhing face">
> >+<!ENTITY smiley6Cmd.tooltip "Insert a embarrassed face">
> >+<!ENTITY smiley7Cmd.tooltip "Insert a undecided face">
> >+<!ENTITY smiley8Cmd.tooltip "Insert a surprise face">
> >+<!ENTITY smiley10Cmd.tooltip "Insert a yell face">
> >+<!ENTITY smiley14Cmd.tooltip "Insert a innocent face">
> >+<!ENTITY smiley15Cmd.tooltip "Insert a cry face">
> 
> I believe these should be:
> 
> +<!ENTITY smiley5Cmd.tooltip "Insert a laughing face">
> +<!ENTITY smiley6Cmd.tooltip "Insert an embarrassed face">
> +<!ENTITY smiley7Cmd.tooltip "Insert an undecided face">
> +<!ENTITY smiley8Cmd.tooltip "Insert a surprised face">
> +<!ENTITY smiley10Cmd.tooltip "Insert a yelling face">
> +<!ENTITY smiley14Cmd.tooltip "Insert an innocent face">
> +<!ENTITY smiley15Cmd.tooltip "Insert a crying face">

Comment 71

13 years ago
Created attachment 166761 [details] [diff] [review]
Patch with Steve Chapel's comment

correct some words I used carelessly

Updated

13 years ago
Attachment #166761 - Flags: superreview?(daniel)

Updated

13 years ago
Attachment #166761 - Flags: superreview?(daniel) → superreview?(Henry.Jia)

Updated

13 years ago
Attachment #166761 - Flags: review+

Comment 72

13 years ago
*** Bug 272848 has been marked as a duplicate of this bug. ***

Comment 73

13 years ago
*** Bug 232428 has been marked as a duplicate of this bug. ***

Comment 74

13 years ago
*** Bug 252192 has been marked as a duplicate of this bug. ***

Comment 75

13 years ago
*** Bug 244421 has been marked as a duplicate of this bug. ***

Comment 76

13 years ago
*** Bug 117377 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Blocks: 209314

Comment 77

13 years ago
*** Bug 133788 has been marked as a duplicate of this bug. ***

Comment 78

13 years ago
Mozilla 1.8 alpha builds have a further problem. The bogus tooltip on a menu
item disappears after a fraction of a second, at which time the menu item loses
focus. This is bug 254224.

Comment 79

13 years ago
*** Bug 209662 has been marked as a duplicate of this bug. ***

Updated

13 years ago
OS: Windows 2000 → All
Hardware: PC → All
Summary: tooltips of menu-buttons appear on the dropdown menuitems → wrong (last or empty) tooltip text displayed for dropdown list menu items

Updated

13 years ago
Attachment #159380 - Attachment is obsolete: true

Comment 80

13 years ago
Comment on attachment 166761 [details] [diff] [review]
Patch with Steve Chapel's comment

sr=Henry
Attachment #166761 - Flags: superreview?(Henry.Jia) → superreview+
Comment on attachment 159380 [details] [diff] [review]
Define very tips in the UI

patch is obsolete
Attachment #159380 - Flags: superreview?(daniel)

Comment 82

13 years ago
checked in
Sandy, please verify it.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 83

13 years ago
The last patch is a workaround for a few cases, but it doesn't fix this bug.
Have a look at the the attached testcases. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 84

13 years ago
*** Bug 250614 has been marked as a duplicate of this bug. ***

Comment 85

13 years ago
*** Bug 279927 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Blocks: 163993

Comment 86

12 years ago
*** Bug 291634 has been marked as a duplicate of this bug. ***

Comment 87

12 years ago
*** Bug 236096 has been marked as a duplicate of this bug. ***

Comment 88

12 years ago
(In reply to comment #47)
> 1) (re)load the testcase, hover on the yellow box wait until the tooltip
> appears, then hover downwards on the orange box: the tooltip text is not
> updated.

I found this bug when having a problem with this behaviour. However, this also
happens on (for example) statusbarpanels (I have a testcase myself, but I think
the one present on this bug will do fine). 
I do think this bug needs an adjustment to its summary - the problems are bigger
than just the dropdownlists.


Comment 89

12 years ago
Created attachment 183543 [details] [diff] [review]
Get tooltiptext a little better than before

This fixes the blank tooltips and "last seen" tooltips you get when hovering
over an item that has tooltiptext set on an ancestor.

Updated

12 years ago
No longer blocks: 163993

Updated

12 years ago
Attachment #183543 - Flags: superreview?(neil.parkwaycc.co.uk)
(Assignee)

Comment 90

12 years ago
Comment on attachment 183543 [details] [diff] [review]
Get tooltiptext a little better than before

Ah yes, I think I have a C++ version of this lying around somewhere...
Attachment #183543 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(Assignee)

Comment 91

12 years ago
Created attachment 185259 [details] [diff] [review]
C++ version
Attachment #185259 - Flags: superreview?(bryner)
Attachment #185259 - Flags: review?(timeless)

Updated

12 years ago
Assignee: sandy.sun → silver
Status: REOPENED → NEW

Comment 92

12 years ago
Comment on attachment 185259 [details] [diff] [review]
C++ version

>Index: layout/xul/base/src/nsXULTooltipListener.cpp
>@@ -548,45 +548,44 @@
> nsXULTooltipListener::GetTooltipFor(nsIContent* aTarget, nsIContent** aTooltip)
>+          return NS_OK;
>         } else {
please resolve the else after return
Attachment #185259 - Flags: review?(timeless) → review+
(Assignee)

Comment 93

12 years ago
Created attachment 185621 [details] [diff] [review]
diff -w
Attachment #185621 - Flags: review?(timeless)

Updated

12 years ago
Attachment #185621 - Flags: review?(timeless) → review+

Comment 94

12 years ago
*** Bug 302844 has been marked as a duplicate of this bug. ***
*** Bug 302845 has been marked as a duplicate of this bug. ***

Comment 96

12 years ago
*** Bug 303318 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Flags: blocking1.8b5?
*** Bug 307146 has been marked as a duplicate of this bug. ***

Comment 98

12 years ago
*** Bug 307268 has been marked as a duplicate of this bug. ***

Comment 99

12 years ago
This happens on trunk and branch all the time for me. Bring cursor on the back
button and wait for the "Go back one page" tooltip to appear. Then go down the
search box on any search engine and wait for the tooltip to appear, it will
still say "Go back one page". Not sure why this still isn't fixed but its a big
visual thing.

Comment 100

12 years ago
*** Bug 307719 has been marked as a duplicate of this bug. ***

Comment 101

12 years ago
*** Bug 292082 has been marked as a duplicate of this bug. ***

Comment 102

12 years ago
*** Bug 305138 has been marked as a duplicate of this bug. ***
*** Bug 307799 has been marked as a duplicate of this bug. ***
*** Bug 308027 has been marked as a duplicate of this bug. ***
*** Bug 308054 has been marked as a duplicate of this bug. ***

Comment 106

12 years ago
*** Bug 276622 has been marked as a duplicate of this bug. ***
*** Bug 308323 has been marked as a duplicate of this bug. ***

Comment 108

12 years ago
We wouldn't stop the release for this. IF a fix gets reviewed and checked into
the trunk in the next couple of days we might consider taking the patch but it
is doubtful this late in the game. 
Flags: blocking1.8b5? → blocking1.8b5-
*** Bug 309357 has been marked as a duplicate of this bug. ***
*** Bug 310011 has been marked as a duplicate of this bug. ***
Attachment #185621 - Flags: approval1.8b5?
Attachment #185621 - Flags: approval1.7.13?
Any reason this has been sitting in limbo since June with a review+ patch?
Attachment #185621 - Flags: approval1.7.13? → approval1.7.13-

Updated

12 years ago
Attachment #185621 - Flags: approval1.8b5? → approval1.8b5-

Updated

12 years ago
Assignee: silver → neil.parkwaycc.co.uk
(Assignee)

Updated

12 years ago
Attachment #185621 - Flags: superreview?(jst)
Comment on attachment 185621 [details] [diff] [review]
diff -w

            mRootBox->GetDefaultTooltip(aTooltip);
-           NS_IF_ADDREF(*aTooltip);
+          if (*aTooltip) {
+            NS_ADDREF(*aTooltip);
+            (*aTooltip)->SetAttr(kNameSpaceID_None, nsXULAtoms::label, tooltipText, PR_TRUE);
+          }

This change looks fine in general, but I can't let this go in w/o fixing the above refcount bug waiting to happen (sorry, not your fault, I know). GetDefaultTooltip() *must* addref its out param and none of its callers (3 currently) should do additional refcounting. This violates the basic COM rules, at no gain, and makes this impossible to use w/ nsCOMPtr.

I'll happily sr this patch if you include that fix in here.
Attachment #185621 - Flags: superreview?(jst) → superreview-
(Assignee)

Comment 113

12 years ago
Created attachment 201613 [details] [diff] [review]
Additional nsIRootBox deCOMtamination
Attachment #185259 - Attachment is obsolete: true
Attachment #185621 - Attachment is obsolete: true
Attachment #201613 - Flags: superreview?(jst)
Attachment #201613 - Flags: review?(jst)
Attachment #185259 - Flags: superreview?(bryner)
Comment on attachment 201613 [details] [diff] [review]
Additional nsIRootBox deCOMtamination

- In nsXULTooltipListener::GetTooltipFor():

-                nsCOMPtr<nsIContent> tooltipContent(do_QueryInterface(tooltipEl));
-                *aTooltip = tooltipContent;
-                NS_IF_ADDREF(*aTooltip);
+            nsCOMPtr<nsIContent> tooltipContent(do_QueryInterface(tooltipEl));
+            *aTooltip = tooltipContent;
+            NS_IF_ADDREF(*aTooltip);

You didn't change this either, but since you're here, wanna replace that assign+addref with a tooltipContent.swap(*aTooltip) to eliminate the extra refcounting?

r+sr=jst, thanks for taking the extra step!
Attachment #201613 - Flags: superreview?(jst)
Attachment #201613 - Flags: superreview+
Attachment #201613 - Flags: review?(jst)
Attachment #201613 - Flags: review+
(Assignee)

Comment 115

12 years ago
I checked this in last night.
Status: NEW → RESOLVED
Last Resolved: 13 years ago12 years ago
Resolution: --- → FIXED

Comment 116

12 years ago
(In reply to comment #108)
> We wouldn't stop the release for this. IF a fix gets reviewed and checked into
> the trunk in the next couple of days we might consider taking the patch but it
> is doubtful this late in the game. 

Is there any chance of this making Firefox RC2? I'm in no position to comment on the risk of the change, but it's a fairly obvious polish bug that makes many tooltips downright wrong (for example, hover over any toolbar button, then over the Google image in the search box...).
Flags: blocking1.8rc2?

Comment 117

12 years ago
(In reply to comment #116)
> (In reply to comment #108)
> > We wouldn't stop the release for this. IF a fix gets reviewed and checked into
> > the trunk in the next couple of days we might consider taking the patch but it
> > is doubtful this late in the game. 
> 
> Is there any chance of this making Firefox RC2? I'm in no position to comment
> on the risk of the change, but it's a fairly obvious polish bug that makes many
> tooltips downright wrong (for example, hover over any toolbar button, then over
> the Google image in the search box...).
> 

Just noticed this. It really shouldn't ship a release with that blunder.

Comment 118

12 years ago
Unfortunate blehmish - but given a ) are about to ship rc2 and a) this has existed for a long time and fixing this has been difficult we are going to hold on this for now.  
Flags: blocking1.8rc2? → blocking1.8rc2-
Flags: blocking1.8rc2-
Flags: blocking1.8b5-
Flags: blocking-aviary2?
Flags: blocking-aviary1.0-

Comment 119

12 years ago
Should this be marked as wontfix then, or is there another bug to fix this in a future branching? Just wondering.

Comment 120

12 years ago
(In reply to comment #119)
> Should this be marked as wontfix then, or is there another bug to fix this in a
> future branching? Just wondering.

RESOLVED FIXED means it has been fixed on the development trunk - all future release branches made from the trunk will automatically have the fix, which essentially means we're done here :-)

WONTFIX, on the contrary, would have meant it will never get fixed on the trunk or any branch. See https://bugzilla.mozilla.org/page.cgi?id=fields.html#status

Comment 121

12 years ago
in tolbar button type="menu-button" i still see the button tooltiptext, when i'm hover with the mouse on the menu. i don't see the menu entry tooltip.
if i remove the tooltip from the button , i can see the tooltip in the menu...??
(Assignee)

Comment 122

12 years ago
(In reply to comment #121)
>in toolbar button type="menu-button" i still see the button tooltiptext, when
>i hover with the mouse on the menu. i don't see the menu entry tooltip. if i
>remove the tooltip from the button, i can see the tooltip in the menu...??
As far as I understand it, this bug is about an incorrect tooltip displayed when the menu entry has no tooltiptext. If you believe that tooltiptext on the menu entry used to work and it no longer does, please attach a test case.

Comment 123

12 years ago
The difference in behavior that I see (TB 1.6a1-1109) is that the tooltip for an item in a menu-button's menu is always the same as the tooltip for the button.  This is an improvement over the previous behavior, which would display the text of the last tooltip shown.

Comment 124

12 years ago
*** Bug 316309 has been marked as a duplicate of this bug. ***

Comment 125

12 years ago
*** Bug 320239 has been marked as a duplicate of this bug. ***

Comment 126

12 years ago
Created attachment 207079 [details]
testcase for tooltiptext on menuitems

> As far as I understand it, this bug is about an incorrect tooltip displayed
> when the menu entry has no tooltiptext.
Right, and that's fixed.

> If you believe that tooltiptext on the menu entry used to work
> and it no longer does, please attach a test case.
Here you are. The tooltiptext of the menuitems is ignored on trunk builds; it shows the tooltiptext of the toolbarbutton ("Print this page"). This works fine with e.g. Mozilla 1.7.12.
Attachment #137368 - Attachment is obsolete: true
So if this bug is fixed, and introduced a regression (which I think is what you're saying, Steffen), then please open a new bug and post the testcase there?
No longer blocks: 209314
(Assignee)

Comment 128

12 years ago
Created attachment 207282 [details] [diff] [review]
This fixes attachment 207079 [details] for some reason?
Attachment #207282 - Flags: superreview?(jst)
Attachment #207282 - Flags: review?(jst)
Comment on attachment 207282 [details] [diff] [review]
This fixes attachment 207079 [details] for some reason?

r+sr=jst
Attachment #207282 - Flags: superreview?(jst)
Attachment #207282 - Flags: superreview+
Attachment #207282 - Flags: review?(jst)
Attachment #207282 - Flags: review+
(Assignee)

Comment 130

12 years ago
Attachment 207282 [details] [diff] checked in.

Comment 131

12 years ago
*** Bug 322605 has been marked as a duplicate of this bug. ***

Comment 132

11 years ago
If the fix causes no known regressions on the trunk, could we get this very frequently duplicated bug fixed on the branch(es) too?
Flags: blocking1.8.1?
Flags: blocking1.8.0.2?
out of scope for the security/stability release.
Flags: blocking1.8.0.2? → blocking1.8.0.2-
Blocker: tooltips need love.  Branch patch needed ASAP.
Flags: blocking1.8.1? → blocking1.8.1+
(Assignee)

Comment 135

11 years ago
jst, which of attachment 185259 [details] [diff] [review] and attachment 201613 [details] [diff] [review] do I need to combine with attachment 207282 [details] [diff] [review] for the branch?

Updated

11 years ago
Blocks: 337169

Comment 136

11 years ago
Looks like this isn't going to happen for beta1.
Flags: blocking1.8.1+ → blocking1.8.1-
Whiteboard: [ff2b2]

Updated

11 years ago
Flags: blocking1.8.1- → blocking1.8.1+
Whiteboard: [ff2b2]
Target Milestone: --- → mozilla1.8.1beta2
*** Bug 344702 has been marked as a duplicate of this bug. ***
Too late for taking this bug, we've mostly worked around this in Fx2 branch, so we don't need the fix except as a platform-level fix, and the amount of work to refactor the patch should be better spent elsewhere.
Flags: blocking1.8.1+ → blocking1.8.1-
Can you at least write an MDC article to describe the workaround, so that extension authors who care about meeting our level of polish don't have to re-invent it from scratch?  I guess that's all I can really ask for at this point...

Comment 140

11 years ago
(In reply to comment #139)
> Can you at least write an MDC article to describe the workaround, so that
> extension authors who care about meeting our level of polish don't have to
> re-invent it from scratch?  I guess that's all I can really ask for at this
> point...
> 

Users of our LibX extension (http://libx.org/) who are using the extension in Firefox 1.5.0.4 are complaining about this bug.

Interesting that such a bug can exist in Mozilla's database for 4 years, be marked "RESOLVED FIXED", yet is entirely ignored.

 - Godmar
If by "entirely ignored" you mean "fixed on the trunk via a series of hard-fought patches dating back to 2002, clearly visibly above were you typed your comment", I suppose so.  RESOLVED FIXED does not mean that the bug fix is available in a particular released version, it means that the fix has been committed to the trunk.  This fix will be included in the Firefox 3 release, according to current plans.

Comment 142

11 years ago
(In reply to comment #141)
> If by "entirely ignored" you mean "fixed on the trunk via a series of
> hard-fought patches dating back to 2002, clearly visibly above were you typed
> your comment", I suppose so.  RESOLVED FIXED does not mean that the bug fix is
> available in a particular released version, it means that the fix has been
> committed to the trunk.  This fix will be included in the Firefox 3 release,
> according to current plans.
> 

Keep in mind that extension authors don't care too much about what fix is on which trunk or branch; all that counts is the released product, and that's Firefox (look at your webserver statistics if you don't believe me.)

The patches linked from this page are all patches to C++ code are they not - could you specifically point at the work-around extension authors are supposed to use?  (I know that you asked for an MDC article, but I would even be willing to read a patch and reimplement it as long as it is something I can do in an extension.)  Mike Connor's comment "we mostly worked around this" isn't too clear on what action extension authors need to take to not be bitten by this. 

I tried the obvious, which is to set tooltiptext="", however, this shows an empty yellow rectangle.  Although this behavior is visually slightly better than displaying the "ghost" tooltips from wherever the user's mouse happened to hover last, it still looks like somebody's first programming project.

 - Godmar
I know that extension authors don't care, but someone who's going to make judgmental commentary in bugzilla should learn what bug states mean before they start throwing stones.  I asked for an MDC article about the necessary workarounds, and I'll follow that up.  You could look at how correctly-behaving pieces of app code work if you'd like to pick that up instead.  Otherwise, you're not adding value here, and I'll ask you to send further commentary or advocacy to me via email.

Comment 144

11 years ago
*** Bug 351158 has been marked as a duplicate of this bug. ***

Comment 145

11 years ago
Is there an MDC article about the necessary workarounds or a link to some sample code that avoids the problem?

Comment 146

10 years ago
Has this bug resurfaced? I'm using Firefox 2.0.0.2 under Windows Vista and am getting this exact problem.

Comment 147

10 years ago
It appears this has never landed on any branches, so you shouldn't expect this to work in 1.5.0.x or 2.0.0.x. Updating target milestone appropriately.
Target Milestone: mozilla1.8.1beta2 → mozilla1.9alpha1

Updated

10 years ago
Blocks: 374288

Updated

10 years ago
Duplicate of this bug: 386581
Duplicate of this bug: 392215

Comment 150

10 years ago
I'm an extension author, and this bug is affecting me.  The comments seem to suggest that there is a work around available, but I can't tell what.  Seconding the request to have some information available here to let extension authors know how to work around the problem.
(In reply to comment #150)

> I'm an extension author, and this bug is affecting me.  The comments seem to
> suggest that there is a work around available, but I can't tell what. 

I'm an extension author too, and I set the right tooltiptext on the menuitem from my overlaid xul, since that menu and tooltip is static. You can also do this dynamically from js or remove the tooltiptext attribute alltogether.

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: timeless → xptoolkit.widgets

Updated

8 years ago
Alias: tooltips
You need to log in before you can comment on or make changes to this bug.