311653 - Icon and dropdown in Search Bar has incorrect (stale) tooltip

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Sahil Sapre]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051006 Firefox/1.4.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051006 Firefox/1.4.1

On http://bugzilla.mozilla.org as well as http://www.gmail.com, hovering the
mouse cursor over the lock in the Address Bar shows the "Signed By" box, which
is expected. But further hovering it over the logo of the search engine being
used in the search box shows the same thing instead of "Search [Search Engine]",
as should have happened

Reproducible: Always

Steps to Reproduce:
1.Go to any https:// (secure) website.
2.Hover mouse over the lock in the Address Bar.
3.Further hover it over the logo of the search engine.

Actual Results:  
"Signed By [name]" box was shown

Expected Results:  
"Search [Search Engine]" should have been seen

Comment 1

[Jesse Ruderman]

Confirmed using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1)
Gecko/20051006 Firefox/1.6a1.

Easier steps to reproduce:

1. Hover over the proxy icon.  
  Tooltip: "Drag and drop this icon to..."

2. Hover over the search bar icon.  
  Tooltip: "Drag and drop this icon to..."
  Should be: "Search Google" or "Click to select a search engine".

Comment 2

[O. Atsushi (Torisugari)]

Attachment: Patch

>-	    this.setAttribute("tooltiptext", toolTipText);
>+	    this.parentNode.setAttribute("tooltiptext", toolTipText);

|this| is a textbox and |this.parentNode| is the entire search box.
tooltip value should cover both textbox and icon.

The other part is just removing the XXX comment.

Comment 3

[Phil Ringnalda (:philor)]

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

Comment 4

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Also see bug 147670

Comment 5

[O. Atsushi (Torisugari)]

Requesting blocking1.8rc1, because this patch is a very simple workaround,
and the incorrect text would be invisible anyway.

There should be a better solution for trunk, e.g. patch for bug 147670 or
"Click to select a search engine" (per comment 1, l10n impact apparently).

Comment 6

[Allan Beaufour]

Checked in on 1.8 branch and trunk

Comment 7

[Phil Ringnalda (:philor)]

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

Comment 8

[Scott MacGregor]

this patch caused a UI regression and was backed out of the branch. 

Comment 9

[O. Atsushi (Torisugari)]

Attachment: Patch (workaround)

>            this.setAttribute("tooltiptext", toolTipText);
> +          /* Tooltip for the icon area */
> +          document.getAnonymousElementByAttribute(this.parentNode, "id",
> +                                                  "searchbar-dropmarker")
> +                  .setAttribute("tooltiptext", toolTipText);

Setting the tooltip to the textbox and to the icon separately, in order
not to triger bug 313673.


At this point, the best thing we can do for the dropdown menu is
to assign the same value as label's, I think. They look a bit
silly though.

That is,
> +       tooltiptext="rdf:http://home.netscape.com/NC-rdf#Name"
>         label="rdf:http://home.netscape.com/NC-rdf#Name"/>
and
> +       element.setAttribute("tooltiptext", label);

Note: separator in the dropdown also had the incorrect tooltip.

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 200839 [details] [diff] [review]
Patch (workaround)

We need to understand and fix the core toolkit problem here and not be using workarounds that might break other tooltips. Is there another bug on file for that?

Comment 11

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #10)
> (From update of attachment 200839 [details] [diff] [review] [edit])
> We need to understand and fix the core toolkit problem here and not be using
> workarounds that might break other tooltips. Is there another bug on file for
> that?

If by "other tooltips" you mean tooltips outside of the search bar, there is no chance that this patch will break them. There is bug 81615 filed on the positioning of inherited tooltips in popups, but that bug only causes problems with the original patch in this bug, which itself has other problems (not all elements in the search bar should have the same tooltip). The latest patch fixes that issue and works around bug 81615 by putting specific tooltips on each of the elements of the search bar instead of relying on one inherited value for the entire search bar. It's a trunk fix, clearly not something worth respinning for.

Comment 12

[Mike Connor [:mconnor]]

Comment on attachment 200839 [details] [diff] [review]
Patch (workaround)

let's get this on the trunk, its not that big of a bug.

Comment 13

[O. Atsushi (Torisugari)]

Attachment: Patch for trunk

Now I don't have to care about l10n thingy, right?

Sooner or later, we need a dtd file for this widget, so I stopped
crappy dynamic menuitem creation/delesion and set label/tooltip
in stable manner.

-          element = document.createElementNS(XUL_NS, "menuseparator");
-          target.appendChild(element);
-          element = document.createElementNS(XUL_NS, "menuitem");
-          element.setAttribute("label", this.mStringBundle
-              .getString("cmd_addEngine"));
-          element.setAttribute("accesskey", this.mStringBundle
-              .getString("cmd_addEngine_accesskey"));
-          element.setAttribute("anonid", "addengine-menuitem");
-          target.appendChild(element);
and
-          var target = aEvent.target;
-          var element =document.getAnonymousElementByAttribute(this.parentNode,
-                           "anonid", "addengine-menuitem");
-          if (element) {
-            target.removeChild(target.lastChild);
-            target.removeChild(target.lastChild);
-          }

+            <xul:menuseparator/>
+            <xul:menuitem label="&cmd_addEngine.label;"
+                          accesskey="&cmd_addEngine.accesskey;"
+                          tooltiptext="&cmd_addEngine.tooltip;"
+                          oncommand="this.parentNode.parentNode.parentNode
+                                         .addEngines();"/>

Comment 14

[O. Atsushi (Torisugari)]

Attachment: Patch (safer workaround for 1.8 branch)

One of this bug trigers is that the icon button is a child element of
the textbox. So setting tooltip for html input element should be a
safer plan.

In this case, we have no tooltip for icon and dropdown.

Comment 15

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

Comment 16

[Mike Schroepfer]

Comment on attachment 201344 [details] [diff] [review]
Patch (safer workaround for 1.8 branch)

Given the previous regressions on this area and only a couple days away from freeze - we are going to hold on this for rc2.

Comment 17

[Scott MacGregor]

we minused this bug  yesterday.

Comment 18

[Zibi Braniecki [:zbraniecki][:gandalf]]

Oh, I'm sorry.

Comment 19

[Teoh Han Hui]

I just encountered this bug today, there must be a way to fix it. Keep up your good work, thanks!

Hope this will get fixed in RC2.

I don't know anything, but just wondering will adding the tooltiptext up one level to cover both the icon and engine name work? Then the separator and "Add Engines..." shouldn't have any tooltiptext. By the way, the tooltiptext for the icon of the selected search engine should follow the tooltiptext of the search field.

Actually, I believe the best way to correct all these is to remove the tooltiptext once the mouse leaves the item that triggered it. (As in the Bookmarks Toolbar and the Tab bar?? They don't cause this problem anyway.)

Comment 20

[Sahil Sapre]

RC2 still doesn't fix this problem. Interestingly, now if a search engine OTHER than Google is selected from the list, the text still says 'Search Google'. I'm not sure that was a problem in previous versions.

Comment 21

[Sokraates]

This bug's still here. It seems that the tool-tip for the search-engine inherits the last shown tool-tip. e.g.: hovering over "Reload current page" or "Go back one page" will show the same tool-tip when afterwards hovering over the search engine. 

It stays the same after selecting another engine. Interestingly, in the drop-down-menu the tool-tip for every single engine will show the "inherited" tip. Also when hovering over "Add engines".

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8) Gecko/20051121 Firefox/1.5 ID:2005112104

Comment 22

[Worcester12345]

I am getting a tooltip error on branch. It just shows a little box maybe 1/32 of an inch square on a 1024x768 monitor.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051130 Firefox/1.5 ID:2005113003

Comment 23

[Sahil Sapre]

1.5 has come and gone. I'm stunned that this bug has gone unnoticed. For the first major release since 1.0, a perfectionist attitude was necessary. Such an obvious bug being put out there in the open is not the first thing that switchers want to see.

Comment 24

[Phil Ringnalda (:philor)]

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

Comment 25

[Ryan Flint [:rflint] (ping via IRC for reviews)]

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

Comment 26

[Jeffrey Kaplan]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

Tooltip for the Search Bar shows the last tooltip shown from anywhere else in Firefox.

Comment 27

[Terry Nightingale]

This could be a related bug or something different.  I didn't see another bug filed when I searched.  When I click the search engine drop down, every search engine in the drop-down list shows the tooltip of "search del.icio.us tag" (one of my search engines), not the tooltip of the search engine I'm hovering over.  This keeps happening even after I choose another search engine and do a search.

Comment 28

[Sokraates]

(In reply to comment #27)
> This could be a related bug or something different.  I didn't see another bug
> filed when I searched.  When I click the search engine drop down, every search
> engine in the drop-down list shows the tooltip of "search del.icio.us tag" (one
> of my search engines), not the tooltip of the search engine I'm hovering over. 
> This keeps happening even after I choose another search engine and do a search.
> 

This is just the same bug. It is best described by Jeffrey Kaplan:

" ------- Comment #26 From Jeffrey Kaplan  2005-12-06 20:22 PST  [reply] -------

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111
Firefox/1.5

Tooltip for the Search Bar shows the last tooltip shown from anywhere else in
Firefox."

After hovering your mouse-pointer over the "Reload"-button, the searchbar will show "Reload current page". Then you can go back to the "Homepage"-button and afterwards the searchbar will misteriously show your home-URL.

Comment 29

[:Gavin Sharp [email: [e-mail]]]

Checked in the initial patch on the branch to sync branch/trunk. These later patches are obsoleted by the patch in bug 317107.
mozilla/browser/base/content/search.xml; new revision: 1.25.2.12;

Comment 30

[[e-mail]]

Bug 147670 should have fixed this (on the trunk, at least).

Comment 31

[hhh]

(In reply to comment #30)
> Bug 147670 should have fixed this (on the trunk, at least).
> 

An old bug has come back. Between the Feb. 23 and Feb. 24 nightlies, Bug 313673 has resurfaced, both on trunk and branch.

Steps to reproduce...
1. Click on the search engine icon.
2. Hover over an icon in the dropdown (ie. eBay).
3. Currently used engine's tooltip appears on far left side of screen (ie. "Search Google").

Comment 32

[Jesse Ruderman]

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

Comment 33

[:Gavin Sharp [email: [e-mail]]]

Attachment: add tooltip to all engines in the dropdown

Fixes bug described in comment 31, at least on the trunk.

Comment 34

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/components/search/content/search.xml 	1.45

Comment 35

[Bryan]

Has the latest patch been checked into the branch?  Still seeing BUG noted in Comment 31 in the latest branch builds!

BUILD: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20060506 BonEcho/2.0a1 ID:2006050614

~B

Comment 36

[:Gavin Sharp [email: [e-mail]]]

Attachment: remove tooltips

Since tooltips on the branch are basically broken (bug 147670 isn't fixed there, and it's patch isn't suitable for the branch), this just removes them all.

Comment 37

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 223577 [details] [diff] [review]
remove tooltips

Mark, can you test this patch to make sure that the search bar is still exposed to screen readers correctly? Aaron mentioned this may be an issue. If it does cause problems, suggestions on how to fix it would be appreciated.

Comment 38

[Mike Connor [:mconnor]]

Comment on attachment 223577 [details] [diff] [review]
remove tooltips

r+a=me, as long as it doesn't regress a11y

Comment 39

[Pam Greene]

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

Comment 40

[:Gavin Sharp [email: [e-mail]]]

Checked in attachment 223577 [details] [diff] [review] on the branch.

Comment 41

[pal-moz]

(In reply to comment #40)
> Checked in attachment 223577 [details] [diff] [review] [edit] on the branch.
> 

remove from Minefield, too ?

Comment 42

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #41)
> remove from Minefield, too ?

Why? They work fine there, with the exception of bug 336231, which can easily be fixed without removing tool tips entirely.

Comment 43

[pal-moz]

(In reply to comment #42)
> (In reply to comment #41)
> > remove from Minefield, too ?
> 
> Why? They work fine there, with the exception of bug 336231, which can easily
> be fixed without removing tool tips entirely.
> 

only for/from Bon Echo ?
In the future, it will be removed from Minefield ?

Comment 44

[:Gavin Sharp [email: [e-mail]]]

Tooltips on the 1.8 branch are broken, so they were removed by attachment 223557 [details] [diff] [review]. Tooltips on the trunk have been fixed in bug 147670. That bug's patch is not suitable for the branch. I don't see any reason to remove tooltips on the trunk.

Comment 45

[:Gavin Sharp [email: [e-mail]]]

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