406456 - Firefox site button should have only one tooltip

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Gabriel Chadwick]

Attachment: Screenshot

Firefox button should have only one tooltip when on unsecured website or in minimized mode. Like current setting on the trunk. To tooltips for the same button is confusing and useless IMO.

Comment 1

[Gabriel Chadwick]

(In reply to comment #0)

> To tooltips for the samebutton is confusing and useless IMO.
> 

I meant two tooltips...

Comment 2

[Ria Klaassen (not reading all bugmail)]

I'm pretty sure that there is already a bug for this.

Comment 3

[Ria Klaassen (not reading all bugmail)]

(In reply to comment #2)
> I'm pretty sure that there is already a bug for this.
>
No sorry, that was Bug 396136. 

Comment 4

[Johnathan Nightingale [:johnath]]

I agree that this is pretty confusing.  Part of the confusion comes from bug 402260 - because the text label that accompanied the site button has been (temporarily) hidden, the non-favicon area is now quite small, leading to the odd behaviour.  If that bug is reversed as intended, then this one should be a non-issue, I'd argue (since we do still want people to know that they can dragdrop the favicon.)

Comment 5

[Gabriel Chadwick]

I understant that this is not an issue on secured website with the text label... This will stay an issue an a unsecured website even if the text label is back. That is why I think it should block.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

True, I can get the behaviour on pages that provide no site information, either.

We should just remove the "drag this favicon" tooltip entirely.

Comment 7

[Gabriel Chadwick]

Is there anyone that disagrees on this change for site without identity information ? If not I think it is more than an easy change and helps for a new function (the site button) discoverability ! Should be made +++ (If only I new how to code)

Comment 8

[Gabriel Chadwick]

Freeze is passed passing to beta4.

Comment 9

[Johnathan Nightingale [:johnath]]

Attachment: Remove "drag favicon" tooltip

(In reply to comment #6)
> True, I can get the behaviour on pages that provide no site information,
> either.
> 
> We should just remove the "drag this favicon" tooltip entirely.
> 

Done.  Removing the associated string as well, I don't *think* that qualifies as late-l10n.

Comment 10

[Gabriel Chadwick]

I know very little about mozilla code but would the best behaviour for the patch be to change the tooltips text to the rest of the site button at the place of removing the tooltip completely ? The site icon should still have a tooltip. The one of the site button IMO.

Comment 11

[Johnathan Nightingale [:johnath]]

(In reply to comment #10)
> I know very little about mozilla code but would the best behaviour for the
> patch be to change the tooltips text to the rest of the site button at the
> place of removing the tooltip completely ? The site icon should still have a
> tooltip. The one of the site button IMO.

Yes, this patch has that effect.  The current code:

http://mxr.mozilla.org/mozilla/source/browser/base/content/browser.js#6452

sets the site-button tooltip on the entire identity-box, including the icon.  However, the hard-coded tooltip on the icon was more-specific, and hence over-riding that tooltip when the mouse was directly over the icon (though the site button tooltip would show up if you hovered *just off* the icon).  By removing the icon's hardcoded tooltip, the one set in code will apply to the entire site button. 

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

The site identity button has become one of the marquee features in Firefox 3. Who knew. This needs to be fixed.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 308475 [details] [diff] [review]
Remove "drag favicon" tooltip

r+uir=beltzner

Comment 14

[Johnathan Nightingale [:johnath]]

tagging late-l10n for the string removal, just in case.

Comment 15

[Johnathan Nightingale [:johnath]]

Checking in browser/base/content/browser.xul;
/cvsroot/mozilla/browser/base/content/browser.xul,v  <--  browser.xul
new revision: 1.445; previous revision: 1.444
done
Checking in browser/locales/en-US/chrome/browser/browser.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.dtd,v  <--  browser.dtd
new revision: 1.103; previous revision: 1.102
done

Spoke to Axel on IRC, confirmed that string removals do not turn l10n trees red, removing late-l10n keyword

Comment 16

[Gabriel Chadwick]

Verified Mozilla/5.0 (Windows; U; Windows NT 6.0; fr; rv:1.9b5pre) Gecko/2008031304 Minefield/3.0b5pre ID:2008031304