Closed
Bug 147670
Opened 22 years ago
Closed 19 years ago
wrong (last or empty) tooltip text displayed for dropdown list menu items
Categories
(Core :: XUL, defect)
Core
XUL
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: sspitzer, Assigned: neil)
References
Details
(Keywords: polish, testcase)
Attachments
(10 files, 6 obsolete files)
128.98 KB,
image/jpeg
|
Details | |
1.12 KB,
patch
|
deanis74
:
review+
bryner
:
superreview-
|
Details | Diff | Splinter Review |
11.91 KB,
patch
|
neil
:
review+
bryner
:
superreview-
|
Details | Diff | Splinter Review |
680 bytes,
patch
|
bryner
:
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
:
superreview+
|
Details | Diff | Splinter Review |
15.98 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
747 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
1.65 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
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).
Reporter | ||
Comment 1•22 years ago
|
||
Reporter | ||
Comment 2•22 years ago
|
||
Reporter | ||
Updated•22 years ago
|
Attachment #85290 -
Attachment is obsolete: true
In fact, it's the last tooltip you had seen. IMO, menuitem is not supposed to
have a tooltip.
That guarantee we'll get the tooltiptext from menuitem itself rather than its
parent - toolbar button.
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
*** Bug 139381 has been marked as a duplicate of this bug. ***
Comment 8•22 years ago
|
||
*** Bug 158280 has been marked as a duplicate of this bug. ***
Updated•22 years ago
|
Keywords: mozilla1.2,
polish
Comment 9•22 years ago
|
||
*** Bug 151270 has been marked as a duplicate of this bug. ***
Comment 10•22 years ago
|
||
*** Bug 146200 has been marked as a duplicate of this bug. ***
Comment 11•22 years ago
|
||
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•22 years ago
|
||
Benjamin, thanks for the verifying. hyatt is working for Apple now, so he
changed his email address.
Comment 13•22 years ago
|
||
Kyle, will this fix bugs like bug 133788 as well?
Comment 15•22 years ago
|
||
oops, it doesn't :( I'll try to make another patch for bug 133788.
No longer blocks: 133788
Comment 16•22 years ago
|
||
*** Bug 186009 has been marked as a duplicate of this bug. ***
Comment 17•22 years ago
|
||
*** Bug 184710 has been marked as a duplicate of this bug. ***
Comment 18•21 years ago
|
||
*** Bug 181617 has been marked as a duplicate of this bug. ***
Comment 19•21 years ago
|
||
*** Bug 161322 has been marked as a duplicate of this bug. ***
Comment 21•21 years ago
|
||
I can still reproduce with the 2003052004 build on Windows 98 with a new profile.
Comment 23•21 years ago
|
||
*** Bug 213244 has been marked as a duplicate of this bug. ***
Comment 24•21 years ago
|
||
*** Bug 201162 has been marked as a duplicate of this bug. ***
Comment 25•21 years ago
|
||
*** Bug 220738 has been marked as a duplicate of this bug. ***
Comment 26•21 years ago
|
||
*** Bug 190199 has been marked as a duplicate of this bug. ***
Comment 27•21 years ago
|
||
*** Bug 228273 has been marked as a duplicate of this bug. ***
Comment 28•21 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•21 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•21 years ago
|
||
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.
Comment 31•21 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•21 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•21 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•21 years ago
|
||
Attachment #137583 -
Flags: superreview?(dbaron)
Attachment #137583 -
Flags: review?(neil.parkwaycc.co.uk)
Assignee: kyle.yuan → timeless
Component: XP Toolkit/Widgets → XP Toolkit/Widgets: XUL
Comment 35•21 years ago
|
||
fwiw my patch can be moved to bug 71329 if people really care...
Comment 36•21 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•21 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•21 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•21 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•21 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 41•21 years ago
|
||
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 42•21 years ago
|
||
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•21 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•21 years ago
|
||
This uses event.preventBubble; would you prefer to use preventDefault?
(in which case the tooltip listener would have to be revised)
Assignee | ||
Updated•21 years ago
|
Attachment #138949 -
Flags: superreview?(bryner)
Comment 45•21 years ago
|
||
*** Bug 231031 has been marked as a duplicate of this bug. ***
Comment 46•21 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•21 years ago
|
||
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•21 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 49•21 years ago
|
||
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-
Comment 50•21 years ago
|
||
yeah, title should "inherit".
Comment 51•21 years ago
|
||
*** Bug 231490 has been marked as a duplicate of this bug. ***
Comment 52•21 years ago
|
||
*** Bug 230694 has been marked as a duplicate of this bug. ***
Comment 53•21 years ago
|
||
*** Bug 232665 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 54•21 years ago
|
||
*** Bug 233286 has been marked as a duplicate of this bug. ***
Comment 55•21 years ago
|
||
Does the fix for Bug 233286 that was checked in today have any effect on this?
Comment 56•21 years ago
|
||
Nope.
Comment 57•21 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•21 years ago
|
||
*** Bug 238424 has been marked as a duplicate of this bug. ***
Comment 59•20 years ago
|
||
*** Bug 246574 has been marked as a duplicate of this bug. ***
Comment 60•20 years ago
|
||
*** Bug 251063 has been marked as a duplicate of this bug. ***
Comment 61•20 years ago
|
||
this is on firefox 0.9 branch
flagging blocking-aviary1.0+
Flags: blocking-aviary1.0+
Comment 62•20 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•20 years ago
|
||
Not blocking 1.0, but a patch would be cool.
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Comment 64•20 years ago
|
||
*** Bug 255475 has been marked as a duplicate of this bug. ***
Comment 65•20 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•20 years ago
|
||
If pointers are dangerous to change,how about define every tips in UI?
Updated•20 years ago
|
Attachment #159380 -
Flags: review?(pete.zha)
Comment 67•20 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•20 years ago
|
Attachment #159380 -
Flags: superreview?(bryner)
Attachment #159380 -
Flags: superreview?(bryner) → superreview?(neil.parkwaycc.co.uk)
Assignee | ||
Comment 68•20 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•20 years ago
|
Attachment #159380 -
Flags: superreview?(daniel)
Comment 69•20 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•20 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•20 years ago
|
||
correct some words I used carelessly
Updated•20 years ago
|
Attachment #166761 -
Flags: superreview?(daniel)
Updated•20 years ago
|
Attachment #166761 -
Flags: superreview?(daniel) → superreview?(Henry.Jia)
Updated•20 years ago
|
Attachment #166761 -
Flags: review+
Comment 72•20 years ago
|
||
*** Bug 272848 has been marked as a duplicate of this bug. ***
Comment 73•20 years ago
|
||
*** Bug 232428 has been marked as a duplicate of this bug. ***
Comment 74•20 years ago
|
||
*** Bug 252192 has been marked as a duplicate of this bug. ***
Comment 75•20 years ago
|
||
*** Bug 244421 has been marked as a duplicate of this bug. ***
Comment 76•20 years ago
|
||
*** Bug 117377 has been marked as a duplicate of this bug. ***
Comment 77•20 years ago
|
||
*** Bug 133788 has been marked as a duplicate of this bug. ***
Comment 78•20 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•20 years ago
|
||
*** Bug 209662 has been marked as a duplicate of this bug. ***
Updated•20 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•20 years ago
|
Attachment #159380 -
Attachment is obsolete: true
Comment 80•20 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 81•20 years ago
|
||
Comment on attachment 159380 [details] [diff] [review]
Define very tips in the UI
patch is obsolete
Attachment #159380 -
Flags: superreview?(daniel)
Comment 82•20 years ago
|
||
checked in
Sandy, please verify it.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 83•20 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•20 years ago
|
||
*** Bug 250614 has been marked as a duplicate of this bug. ***
Comment 85•20 years ago
|
||
*** Bug 279927 has been marked as a duplicate of this bug. ***
Comment 86•19 years ago
|
||
*** Bug 291634 has been marked as a duplicate of this bug. ***
Comment 87•19 years ago
|
||
*** Bug 236096 has been marked as a duplicate of this bug. ***
Comment 88•19 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•19 years ago
|
||
This fixes the blank tooltips and "last seen" tooltips you get when hovering
over an item that has tooltiptext set on an ancestor.
Attachment #183543 -
Flags: superreview?(neil.parkwaycc.co.uk)
Assignee | ||
Comment 90•19 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•19 years ago
|
||
Attachment #185259 -
Flags: superreview?(bryner)
Attachment #185259 -
Flags: review?(timeless)
Comment 92•19 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•19 years ago
|
||
Attachment #185621 -
Flags: review?(timeless)
Attachment #185621 -
Flags: review?(timeless) → review+
Comment 94•19 years ago
|
||
*** Bug 302844 has been marked as a duplicate of this bug. ***
Comment 95•19 years ago
|
||
*** Bug 302845 has been marked as a duplicate of this bug. ***
Comment 96•19 years ago
|
||
*** Bug 303318 has been marked as a duplicate of this bug. ***
Comment 97•19 years ago
|
||
*** Bug 307146 has been marked as a duplicate of this bug. ***
Comment 98•19 years ago
|
||
*** Bug 307268 has been marked as a duplicate of this bug. ***
Comment 99•19 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•19 years ago
|
||
*** Bug 307719 has been marked as a duplicate of this bug. ***
Comment 101•19 years ago
|
||
*** Bug 292082 has been marked as a duplicate of this bug. ***
Comment 102•19 years ago
|
||
*** Bug 305138 has been marked as a duplicate of this bug. ***
Comment 103•19 years ago
|
||
*** Bug 307799 has been marked as a duplicate of this bug. ***
Comment 104•19 years ago
|
||
*** Bug 308027 has been marked as a duplicate of this bug. ***
Comment 105•19 years ago
|
||
*** Bug 308054 has been marked as a duplicate of this bug. ***
Comment 106•19 years ago
|
||
*** Bug 276622 has been marked as a duplicate of this bug. ***
Comment 107•19 years ago
|
||
*** Bug 308323 has been marked as a duplicate of this bug. ***
Comment 108•19 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-
Comment 109•19 years ago
|
||
*** Bug 309357 has been marked as a duplicate of this bug. ***
Comment 110•19 years ago
|
||
*** Bug 310011 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Attachment #185621 -
Flags: approval1.8b5?
Attachment #185621 -
Flags: approval1.7.13?
Comment 111•19 years ago
|
||
Any reason this has been sitting in limbo since June with a review+ patch?
Attachment #185621 -
Flags: approval1.7.13? → approval1.7.13-
Updated•19 years ago
|
Attachment #185621 -
Flags: approval1.8b5? → approval1.8b5-
Updated•19 years ago
|
Assignee: silver → neil.parkwaycc.co.uk
Assignee | ||
Updated•19 years ago
|
Attachment #185621 -
Flags: superreview?(jst)
Comment 112•19 years ago
|
||
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•19 years ago
|
||
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 114•19 years ago
|
||
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•19 years ago
|
||
I checked this in last night.
Status: NEW → RESOLVED
Closed: 20 years ago → 19 years ago
Resolution: --- → FIXED
Comment 116•19 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•19 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•19 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-
Updated•19 years ago
|
Flags: blocking1.8rc2-
Flags: blocking1.8b5-
Flags: blocking-aviary2?
Flags: blocking-aviary1.0-
Comment 119•19 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•19 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•19 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•19 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•19 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•19 years ago
|
||
*** Bug 316309 has been marked as a duplicate of this bug. ***
Comment 125•19 years ago
|
||
*** Bug 320239 has been marked as a duplicate of this bug. ***
Comment 126•19 years ago
|
||
> 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
Comment 127•19 years ago
|
||
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?
Assignee | ||
Comment 128•19 years ago
|
||
Attachment #207282 -
Flags: superreview?(jst)
Attachment #207282 -
Flags: review?(jst)
Comment 129•19 years ago
|
||
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•19 years ago
|
||
Attachment 207282 [details] [diff] checked in.
Comment 131•19 years ago
|
||
*** Bug 322605 has been marked as a duplicate of this bug. ***
Comment 132•19 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?
Comment 133•19 years ago
|
||
out of scope for the security/stability release.
Flags: blocking1.8.0.2? → blocking1.8.0.2-
Comment 134•19 years ago
|
||
Blocker: tooltips need love. Branch patch needed ASAP.
Flags: blocking1.8.1? → blocking1.8.1+
Assignee | ||
Comment 135•19 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?
Comment 136•18 years ago
|
||
Looks like this isn't going to happen for beta1.
Flags: blocking1.8.1+ → blocking1.8.1-
Whiteboard: [ff2b2]
Updated•18 years ago
|
Flags: blocking1.8.1- → blocking1.8.1+
Whiteboard: [ff2b2]
Target Milestone: --- → mozilla1.8.1beta2
Comment 137•18 years ago
|
||
*** Bug 344702 has been marked as a duplicate of this bug. ***
Comment 138•18 years ago
|
||
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-
Comment 139•18 years ago
|
||
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•18 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
Comment 141•18 years ago
|
||
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•18 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
Comment 143•18 years ago
|
||
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•18 years ago
|
||
*** Bug 351158 has been marked as a duplicate of this bug. ***
Comment 145•18 years ago
|
||
Is there an MDC article about the necessary workarounds or a link to some sample code that avoids the problem?
Comment 146•18 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•18 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
Comment 150•17 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.
Comment 151•16 years ago
|
||
(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.
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: timeless → xptoolkit.widgets
Updated•16 years ago
|
Alias: tooltips
You need to log in
before you can comment on or make changes to this bug.
Description
•