Right now, the ambient indicator size and visibility depends on a counter of unread notifications. That could lead to cases in which an external application removes a notification and that makes the ambient indicator not showing the actual number of unread notifications. For fixing that, we should keep a list of unread notifications (instead of a counter), and remove them from the list when they are read.
As an example, here is a use case mhenretty was pointing at: "If I get a download notification (unread=1), then read it (unread=0), then I get an email notification (unread=1), and then the system removes the download notification (decExternalNotifications, unread=0), the ambient indicator will go away even though I have an unread email notification. Instead of a count, what we need is a list of unread notificationId's. Then when we add or remove notifications, we also add remove from this list, and we use this list for the unread count."
Created attachment 8488450 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23982
Comment on attachment 8488450 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23982 Sweet! Left a small nit on github. And let's add a test to make sure the scenario in comment 1 is addressed.
[Blocking Requested - why for this release]: This bug could lead to a bad UX, as the ambient indicator could show an incorrect number of notifications (even missing unread notifications)
Please add a Gaia v2.1 nomination when you get a chance :)
Comment on attachment 8488450 [details] [review] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23982 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): - [User impact] if declined: The ambient indicator could show an incorrect number of notifications (even missing unread notifications) [Testing completed]: Added unit tests. Bug found and fixed on bug 1073032. We should uplift both. [Risk to taking this patch] (and alternatives if risky): It has been in master for a while, and the only bug found was fixed. I think the risk is low compared to the bad ux this bug adds to the system. [String changes made]: -
Reverted from v2.1 for Gaia unit test failures. v2.1: https://github.com/mozilla-b2g/gaia/commit/a00d102abfe8ae15c4fd14771efa2335c6d3b8d9 https://tbpl.mozilla.org/php/getParsedLog.php?id=49158087&tree=Mozilla-Aurora
It was all green on master, so I guess another patch is dependent. I'll take a look. Thanks!
Please verify on 2.1 branch. when done, flip status-b2g-v2.1: "verified"
Verified the issue is fixed on Flame 2.2 and 2.1 The indicator is based on list of notifications and not on a counter Device: Flame 2.2 KK Master BuildID: 20141024040202 Gaia: d893a9b971a0f3ee48e5a57dca516837d92cf52b Gecko: a5ee2769eb27 Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d Version: 36.0a1 (2.2 Master) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 Device: Flame 2.1 KK BuildID: 20141024001204 Gaia: 0f76e0baac733cca56d0140e954c5f446ebc061f Gecko: 7d78ff7d25b6 Gonk: 05aa7b98d3f891b334031dc710d48d0d6b82ec1d Version: 34.0 (2.1) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
This will be in-testsuite+ as soon as https://github.com/mozilla-b2g/gaia/pull/25215 lands