Closed
Bug 1187022
Opened 9 years ago
Closed 9 years ago
Add hover tooltip for shield icon in PBM mode
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
Tracking | Status | |
---|---|---|
firefox42 | --- | verified |
People
(Reporter: agrigas, Assigned: bgrins)
References
Details
(Whiteboard: [fxprivacy][campaign])
Attachments
(1 file)
Final copy on this ticket: https://bugzilla.mozilla.org/show_bug.cgi?id=1186166
Flags: firefox-backlog?
Updated•9 years ago
|
Updated•9 years ago
|
Flags: qe-verify?
Flags: firefox-backlog?
Flags: firefox-backlog+
Assignee | ||
Comment 1•9 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•9 years ago
|
Flags: qe-verify? → qe-verify+
Updated•9 years ago
|
Whiteboard: [fxprivacy] → [fxprivacy] [campaign]
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Points: --- → 2
Assignee | ||
Comment 2•9 years ago
|
||
Bug 1187022 - Add hover tooltip for tracking protection shield icon;r=MattN
Attachment #8638235 -
Flags: review?(MattN+bmo)
Comment 3•9 years ago
|
||
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+
Comment 4•9 years ago
|
||
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•9 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•9 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•9 years ago
|
||
Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d266b205ea21
Assignee | ||
Comment 8•9 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
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][fxprivacy][campaign] → [fxprivacy][campaign]
Target Milestone: --- → Firefox 42
Updated•9 years ago
|
Iteration: --- → 42.2 - Jul 27
Updated•9 years ago
|
QA Contact: mwobensmith
You need to log in
before you can comment on or make changes to this bug.
Description
•