Last Comment Bug 710589 - Close button is still displayed in Telemetry notification when selecting Privacy Policy
: Close button is still displayed in Telemetry notification when selecting Priv...
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 13
Assigned To: Nathan Froyd [:froydnj]
:
Mentors:
Depends on: 697860
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-14 05:21 PST by Virgil Dicu [:virgil] [QA]
Modified: 2012-02-25 03:19 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
this should fix it (1.89 KB, patch)
2011-12-14 17:47 PST, (dormant account)
no flags Details | Diff | Review
patch (3.49 KB, patch)
2012-02-24 09:00 PST, Nathan Froyd [:froydnj]
dietrich: review+
Details | Diff | Review
patch (3.53 KB, patch)
2012-02-24 13:17 PST, Nathan Froyd [:froydnj]
nfroyd: review+
vladan.bugzilla: checkin+
Details | Diff | Review

Description Virgil Dicu [:virgil] [QA] 2011-12-14 05:21:55 PST
Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111208 Firefox/11.0a1

The Telemetry notification still displays the close (x) button when selecting Learn more (which redirects to Privacy Policy) after receiving the two-state notification.
Bug 697860 states that the close button shouldn't be displayed anymore for the Telemetry Notification.

STR:

1. Start Firefox with a clean profile
2. Restart Firefox to dismiss the "Know your rights" notification and receive the two state Telemetry notification
3. Select Learn more. (This will redirect to the Privacy Policy)

The Telemetry close button is still displayed here. The button should be hidden as specified in bug 697860.
Comment 1 Lawrence Mandel [:lmandel] (use needinfo) 2011-12-14 07:05:35 PST
Probably not terrible to display the close button here as long as clicking close != a click on No for Telemetry.
Comment 2 (dormant account) 2011-12-14 17:47:01 PST
Created attachment 581852 [details] [diff] [review]
this should fix it
Comment 3 (dormant account) 2012-02-15 19:20:16 PST
Vladan, can you wrap this up?
Comment 4 Nathan Froyd [:froydnj] 2012-02-23 13:58:34 PST
This will make bug 725407 somewhat less ugly too.
Comment 5 Nathan Froyd [:froydnj] 2012-02-24 09:00:35 PST
Created attachment 600411 [details] [diff] [review]
patch

Revised patch.  I gave the function a number of parameters so that it's useful for bug 725407.
Comment 6 Dietrich Ayala (:dietrich) 2012-02-24 10:59:51 PST
Comment on attachment 600411 [details] [diff] [review]
patch

Review of attachment 600411 [details] [diff] [review]:
-----------------------------------------------------------------

a couple of style nits, r=me otherwise.

::: browser/components/nsBrowserGlue.js
@@ +776,5 @@
>      const PREF_TELEMETRY_SERVER_OWNER = "toolkit.telemetry.server_owner";
>      // This is used to reprompt users when privacy message changes
>      const TELEMETRY_PROMPT_REV = 2;
>  
> +    function appendTelemetryNotification(aNotifyBox, message, buttons, hideclose) {

be consistent w/ prefixing parameters with "a" and camel-casing (eg, compare aNotifyBox with hideclose). look elsewhere in file for what styles to follow here.

@@ +777,5 @@
>      // This is used to reprompt users when privacy message changes
>      const TELEMETRY_PROMPT_REV = 2;
>  
> +    function appendTelemetryNotification(aNotifyBox, message, buttons, hideclose) {
> +      let notification = aNotifyBox.appendNotification(message, "telemetry", null, aNotifyBox.PRIORITY_INFO_LOW, buttons);

80 char line length plz
Comment 7 Nathan Froyd [:froydnj] 2012-02-24 13:17:40 PST
Created attachment 600497 [details] [diff] [review]
patch

Made style consistent by using non-a-prefixed things and broke lines as best I could.  Carrying over r+.
Comment 9 Marco Bonardo [::mak] 2012-02-25 02:29:07 PST
https://hg.mozilla.org/mozilla-central/rev/481410bfe37c

Note You need to log in before you can comment on or make changes to this bug.