1187022 - Add hover tooltip for shield icon in PBM mode

Status: VERIFIED FIXED

(Firefox :: General, defect, P1)

Description

[Ash Grigas]

Final copy on this ticket:
https://bugzilla.mozilla.org/show_bug.cgi?id=1186166

Comment 1

[Brian Grinstead [:bgrins]]

The strings (taken from Bug 1186166 and a brief chat on irc):

If tracking content is detected but is disabled for the site:
“Tracking content detected”

If tracking protection is enabled and blocking content on the site:
“Tracking attempts blocked”

Comment 2

[Brian Grinstead [:bgrins]]

Attachment: MozReview Request: Bug 1187022 - Add hover tooltip for tracking protection shield icon;r=MattN

Bug 1187022 - Add hover tooltip for tracking protection shield icon;r=MattN

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8638235 [details]
MozReview Request: Bug 1187022 - Add hover tooltip for tracking protection shield icon;r=MattN

https://reviewboard.mozilla.org/r/14077/#review12605

::: browser/locales/en-US/chrome/browser/browser.properties:346
(Diff revision 1)
> +trackingProtection.icon.activeTooltip=Tracking attempts blocked.
> +trackingProtection.icon.disabledTooltip=Tracking content detected.

The periods weren't specified in the bug and I don't see periods used on other tooltips so please remove them.

Comment 4

[Matthew N. [:MattN]]

https://reviewboard.mozilla.org/r/14077/#review12607

::: browser/base/content/browser-trackingprotection.js:22
(Diff revision 1)
> +    this.activeTooltipText =
> +      gNavigatorBundle.getString("trackingProtection.icon.activeTooltip");
> +    this.disabledTooltipText =
> +      gNavigatorBundle.getString("trackingProtection.icon.disabledTooltip");

Also, AFAIK we normally define members as properties of the object with default values to make them easier to find. I'm not sure why we haven't been doing that already with `container`, `content`, `icon`, etc. already.


i.e. add `activeTooltipText: null,` with the other properties at the beginning of the object.

You don't need to change this now since we don't seem to be doing that consistently in this object.

Comment 5

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] from comment #3)
> Comment on attachment 8638235 [details]
> MozReview Request: Bug 1187022 - Add hover tooltip for tracking protection
> shield icon;r=MattN
> 
> https://reviewboard.mozilla.org/r/14077/#review12605
> 
> ::: browser/locales/en-US/chrome/browser/browser.properties:346
> (Diff revision 1)
> > +trackingProtection.icon.activeTooltip=Tracking attempts blocked.
> > +trackingProtection.icon.disabledTooltip=Tracking content detected.
> 
> The periods weren't specified in the bug and I don't see periods used on
> other tooltips so please remove them.

Ah interesting, I had added them after viewing the "This website does not supply identity information." tooltip for an http page.  I'll remove them though

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Matthew N. [:MattN] from comment #4)
> https://reviewboard.mozilla.org/r/14077/#review12607
> 
> ::: browser/base/content/browser-trackingprotection.js:22
> (Diff revision 1)
> > +    this.activeTooltipText =
> > +      gNavigatorBundle.getString("trackingProtection.icon.activeTooltip");
> > +    this.disabledTooltipText =
> > +      gNavigatorBundle.getString("trackingProtection.icon.disabledTooltip");
> 
> Also, AFAIK we normally define members as properties of the object with
> default values to make them easier to find. I'm not sure why we haven't been
> doing that already with `container`, `content`, `icon`, etc. already.
> 
> 
> i.e. add `activeTooltipText: null,` with the other properties at the
> beginning of the object.
> 
> You don't need to change this now since we don't seem to be doing that
> consistently in this object.

Went ahead and added all of the properties (including the new ones) as null values on the object literal to be consistent

Comment 7

[Brian Grinstead [:bgrins]]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d266b205ea21

Comment 8

[Brian Grinstead [:bgrins]]

remote:   https://hg.mozilla.org/integration/fx-team/rev/af22172d33b6

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/af22172d33b6

Comment 10

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified fixed on Nightly Fx42.0a1, 2015-07-27.