Add hover tooltip for shield icon in PBM mode

VERIFIED FIXED in Firefox 42

Status

()

Firefox
General
P1
normal
Rank:
1
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: Ash Grigas, Assigned: bgrins)

Tracking

Trunk
Firefox 42
Points:
2
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(firefox42 verified)

Details

(Whiteboard: [fxprivacy][campaign])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
Final copy on this ticket:
https://bugzilla.mozilla.org/show_bug.cgi?id=1186166
Flags: firefox-backlog?

Updated

2 years ago
Rank: 1
Depends on: 1186166
Priority: -- → P1

Updated

2 years ago
Flags: qe-verify?
Flags: firefox-backlog?
Flags: firefox-backlog+
(Assignee)

Comment 1

2 years ago
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”
(Assignee)

Updated

2 years ago
Flags: qe-verify? → qe-verify+

Updated

2 years ago
Whiteboard: [fxprivacy] → [fxprivacy] [campaign]
(Assignee)

Updated

2 years ago
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Points: --- → 2
(Assignee)

Comment 2

2 years ago
Created attachment 8638235 [details]
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
Attachment #8638235 - Flags: review?(MattN+bmo)
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.
Attachment #8638235 - Flags: review?(MattN+bmo) → review+
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.
(Assignee)

Comment 5

2 years ago
(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
(Assignee)

Comment 6

2 years ago
(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
(Assignee)

Comment 7

2 years ago
Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d266b205ea21
(Assignee)

Comment 8

2 years ago
remote:   https://hg.mozilla.org/integration/fx-team/rev/af22172d33b6
Whiteboard: [fxprivacy] [campaign] → [fixed-in-fx-team][fxprivacy][campaign]
https://hg.mozilla.org/mozilla-central/rev/af22172d33b6
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][fxprivacy][campaign] → [fxprivacy][campaign]
Target Milestone: --- → Firefox 42
Verified fixed on Nightly Fx42.0a1, 2015-07-27.
Status: RESOLVED → VERIFIED
status-firefox42: fixed → verified

Updated

2 years ago
Iteration: --- → 42.2 - Jul 27

Updated

2 years ago
QA Contact: mwobensmith
You need to log in before you can comment on or make changes to this bug.