Closed Bug 872543 Opened 10 years ago Closed 10 years ago

Long-form data reporting notification sticks around after visiting settings


(Firefox Health Report Graveyard :: Client: Android, defect, P2)



(firefox23 fixed, firefox24 fixed)

Firefox 24
Tracking Status
firefox23 --- fixed
firefox24 --- fixed


(Reporter: rnewman, Assigned: liuche)




(2 files, 2 obsolete files)

HTC One, 4.1.something, HTC Sense.

Two-finger swiping on the notification expands it, which is awesome.

You now see a gear icon and "Choose what I share".

Tapping that action takes you to settings. It does not dismiss the notification.

Tapping the body of the notification takes you to settings and dismisses the notification.

I thing the former should also dismiss the notification. I was quite surprised to see it still there.
Seems like setAutoCancel() doesn't work as expected on the big notification action. This probably needs an explicit clearing of the notification when click "action" is called.
Seems like this'll be a quick fix...
Assignee: nobody → liuche
This patch explicitly cancels the notification when the notification Action has been selected.

This also uses the correct (white) icon in the BigNotification.
Attachment #752369 - Flags: review?(rnewman)
Comment on attachment 752369 [details] [diff] [review]
Patch: Explicitly cancel notification on preferences launch.

Review of attachment 752369 [details] [diff] [review]:

Looks good.

Another option to consider: always attempt to cancel the notification when the user accesses data reporting preferences. Your call.

Please land, bake, then flag for uplift.
Attachment #752369 - Flags: review?(rnewman) → review+
I considered that, but adding a single extra key into the bundle is pretty trivial and a one-time-call, and I don't like the idea of sending a notification cancel every single time a settings pane is rendered.

Waiting for inbound to re-open...

Will bake on Nightly and if no regressions, will submit an aurora patch.
Target Milestone: --- → Firefox 23
Target Milestone: Firefox 23 → Firefox 24
Backed out in for "java-exception | java.lang.NullPointerExceptionat org.mozilla.gecko.GeckoPreferences.onCreate(" in robocop like
Left out a null check for intent extras! Added that in, waiting on Try tests.
Attachment #752369 - Attachment is obsolete: true
Attachment #753009 - Flags: review?(nalexander)
Comment on attachment 753009 [details] [diff] [review]
Patch: Explicitly cancel notification on preferences launch v2

Review of attachment 753009 [details] [diff] [review]:

::: mobile/android/base/
@@ +56,5 @@
>                  Bundle fragmentArgs = new Bundle();
>                  fragmentArgs.putString("resource", "preferences_datareporting");
>                  prefIntent.putExtra(PreferenceActivity.EXTRA_SHOW_FRAGMENT_ARGUMENTS, fragmentArgs);
>              }

This is weird: you include the notificationId in the extras, but when you actually delete the notification, you ignore it.  Either be consistent and cancel the passed in ID, or change the meaning of the extra to be "if this key is present, cancel the one unique notification ID".  I think I prefer the latter, since it avoids having to parse the extras bundle.
Attachment #753009 - Flags: review?(nalexander) → review-
Changed the value from an id to a boolean, but kept the keyname reuse. I kind of want to create another field like IS_DATAREPORTING_NOTIFICATION, because it'd technically be more correct and descriptive, but it would add yet another datareporting-specific single use field.
Attachment #753009 - Attachment is obsolete: true
Attachment #753022 - Flags: review?(nalexander)
Attachment #753022 - Flags: review?(nalexander) → review+
All green on try with android unit-tests with the null check added.
Closed: 10 years ago
Resolution: --- → FIXED
This has been baking on Nightly for a couple of weeks without problems. Testing on try server against current aurora, then will nom for aurora uplift.
Comment on attachment 758120 [details] [diff] [review]
Aurora patch: Clear notification explicitly on tap

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Follow-up to bug 833625
User impact if declined: Users of Android 4.1+ may see Android system notification for Firefox data collection persist even after attempting to make selections.
Testing completed (on m-c, etc.): This has baked in Nightly since 5/23; passes all tests on try: and runs correctly
Risk to taking this patch (and alternatives if risky): low - added a single additional conditional check for a new flag added in this patch.
String or IDL/UUID changes made by this patch: none
Attachment #758120 - Flags: approval-mozilla-aurora?
Attachment #758120 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.