Closed Bug 311653 Opened 14 years ago Closed 14 years ago

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

Categories

(Firefox :: Search, defect)

2.0 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 2 beta1

People

(Reporter: sahilsapre, Assigned: Gavin)

References

Details

(Keywords: fixed1.8.1, polish)

Attachments

(2 files, 4 obsolete files)

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
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".
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: on www.gmail.com, hovering the mouse cursor over the lock in the address field shows "Signed by Thawte Consulting...". Futher hovering the cursor over the search box shows the same thing instead of "Search [Search Engine]" → Icon in Search Bar has incorrect (stale) tooltip
Attached patch Patch (obsolete) — Splinter Review
>-	    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.
Keywords: polish
OS: Windows XP → All
Hardware: PC → All
*** Bug 311698 has been marked as a duplicate of this bug. ***
Attachment #198929 - Flags: review?(mconnor)
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).
Flags: blocking1.8rc1?
Attachment #198929 - Flags: review?(mconnor) → review+
Attachment #198929 - Flags: approval1.8rc1+
Checked in on 1.8 branch and trunk
Status: NEW → RESOLVED
Closed: 14 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Flags: blocking1.8rc1?
*** Bug 313374 has been marked as a duplicate of this bug. ***
Depends on: 313673
this patch caused a UI regression and was backed out of the branch. 
Keywords: fixed1.8
Attachment #198929 - Flags: approval1.8rc1+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: Icon in Search Bar has incorrect (stale) tooltip → Icon and dropdown in Search Bar has incorrect (stale) tooltip
Attached patch Patch (workaround) (obsolete) — Splinter Review
>            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.
Attachment #198929 - Attachment is obsolete: true
Attachment #200839 - Flags: review?
Attachment #200839 - Flags: approval1.8rc1?
Attachment #200839 - Flags: review? → review?(mconnor)
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?
Attachment #200839 - Flags: approval1.8rc1? → approval1.8rc1-
(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.
Assignee: nobody → torisugari
Status: REOPENED → NEW
Target Milestone: --- → Firefox1.6-
Version: unspecified → Trunk
Comment on attachment 200839 [details] [diff] [review]
Patch (workaround)

let's get this on the trunk, its not that big of a bug.
Attachment #200839 - Flags: review?(mconnor) → review+
Attached patch Patch for trunk (obsolete) — Splinter Review
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();"/>
Attachment #200999 - Flags: review?(mconnor)
Attachment #200839 - Attachment is obsolete: true
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.
*** Bug 314562 has been marked as a duplicate of this bug. ***
Attachment #201344 - Flags: review?(mconnor)
Attachment #201344 - Flags: approval1.8rc2?
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.
Attachment #201344 - Flags: approval1.8rc2? → approval1.8rc2-
Flags: blocking1.8rc2?
we minused this bug  yesterday.
Flags: blocking1.8rc2? → blocking1.8rc2-
Oh, I'm sorry.
Flags: blocking-aviary2?
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.)
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.
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
Flags: blocking-aviary2? → blocking-aviary2+
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
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.
*** Bug 318883 has been marked as a duplicate of this bug. ***
*** Bug 319191 has been marked as a duplicate of this bug. ***
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.
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.
(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.
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;
Depends on: 317107
Attachment #200999 - Attachment is obsolete: true
Attachment #201344 - Attachment is obsolete: true
Attachment #200999 - Flags: review?(mconnor)
Attachment #201344 - Flags: review?(mconnor)
Bug 147670 should have fixed this (on the trunk, at least).
Target Milestone: Firefox 2 → Firefox 2 beta1
(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").
*** Bug 335111 has been marked as a duplicate of this bug. ***
Fixes bug described in comment 31, at least on the trunk.
Assignee: torisugari → gavin.sharp
Status: NEW → ASSIGNED
Attachment #220176 - Flags: review?(mconnor)
Whiteboard: [patch-r?]
Attachment #220176 - Flags: review?(mconnor) → review+
Whiteboard: [patch-r?] → [checkin needed]
mozilla/browser/components/search/content/search.xml 	1.45
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Target Milestone: Firefox 2 beta1 → Firefox 3
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
Attached patch remove tooltipsSplinter Review
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.
Attachment #223577 - Flags: review?(mconnor)
Attachment #223577 - Flags: approval-branch-1.8.1?(mconnor)
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.
Attachment #223577 - Flags: superreview?(pilgrim)
Whiteboard: [patch-r?]
Comment on attachment 223577 [details] [diff] [review]
remove tooltips

r+a=me, as long as it doesn't regress a11y
Attachment #223577 - Flags: review?(mconnor)
Attachment #223577 - Flags: review+
Attachment #223577 - Flags: approval-branch-1.8.1?(mconnor)
Attachment #223577 - Flags: approval-branch-1.8.1+
Target Milestone: Firefox 3 → Firefox 2 beta1
Version: Trunk → 2.0 Branch
Attachment #223577 - Flags: superreview?(pilgrim) → superreview+
Whiteboard: [patch-r?] → [checkin needed (1.8 branch)]
*** Bug 337169 has been marked as a duplicate of this bug. ***
Checked in attachment 223577 [details] [diff] [review] on the branch.
Keywords: fixed1.8.1
Whiteboard: [checkin needed (1.8 branch)]
(In reply to comment #40)
> Checked in attachment 223577 [details] [diff] [review] [edit] on the branch.
> 

remove from Minefield, too ?
(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.
(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 ?
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.
*** Bug 350098 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.