Closed
Bug 1493864
Opened 6 years ago
Closed 6 years ago
Notifications no longer emit sound
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox62 unaffected, firefox63 wontfix, firefox64 fixed)
RESOLVED
FIXED
Firefox 64
Tracking | Status | |
---|---|---|
firefox62 | --- | unaffected |
firefox63 | --- | wontfix |
firefox64 | --- | fixed |
People
(Reporter: ajv-939-807-4355, Assigned: JanH)
References
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
User Agent: Mozilla/5.0 (X11; Linux i686; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20180920135444
Steps to reproduce:
Push notification to browser (permission allowed)
Actual results:
Notification popup is placed, but not sound played
Expected results:
I should hear a sound for the new notification (there's a long-standing bug for the
notification light to start blinking, but that's a different bug(
Assignee | ||
Comment 1•6 years ago
|
||
Can I assume from that that your phone runs Oreo or later? The notifications only did that because we accidentally set the default priority for all notifications on Android O and later to "High", which for most notifications is too much. But true, there's already bug 1236902 for the same basic issue, namely that those notifications don't have a separate priority.
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 2•6 years ago
|
||
If you'll look at bug 1257373 comment 18 from Jovan Gerodetti, notification with sound started working, and I certainly used it in pre-O Android. Like I said, no LED, but yes sound with the notification. But now with the Beta builds it's back to not making a sound? I just re-verified, Firefox 61.0 on Android 7.1.2--phone off, notification comes in, and sound. 63.0b7 on 8.1.0, back to no sound--stealth notifications. However, 62.0.2 on 8.1.0 does indeed do sound with the notification.
So I stand by my bug report--there's a regression, sound *was* and *is* working in 61 and 62, but has stopped working in the beta builds. Please let me know if you need some test case JS code, and I can extract something from my messaging server source code.
Assignee | ||
Comment 3•6 years ago
|
||
You're right, sorry, I didn't read the code closely enough. While we don't set a standard notification priority, sound was indeed added in bug 1378445.
As a workaround, you can go into Android's notification settings and change the settings for our default notification channel (currently the app name, might change to "Browser" in the future) to something audible.
Assignee: nobody → jh+bugzilla
Blocks: 1479532
Status: RESOLVED → REOPENED
status-firefox62:
--- → unaffected
status-firefox63:
--- → affected
status-firefox64:
--- → affected
tracking-firefox63:
--- → ?
Ever confirmed: true
Keywords: regression
OS: Unspecified → Android
Hardware: Unspecified → All
Resolution: DUPLICATE → ---
Assignee | ||
Comment 4•6 years ago
|
||
Flod, creating a new notification channel needs a new string, so what's your opinion regarding fixing this on Beta?
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 5•6 years ago
|
||
In addition to the string changes from bug 1488874, we also need a separate notification channel for web notifications, since those used to be audible and therefore need a separate channel in order to use different default notification settings (also so users can configure the separately).
What should that channel be called: "Site notifications", "Web notifications", or ...?
Also what would sensible default settings for that channel be? Just sound, or sound + notification light, or even vibration? Not sure whether a heads-up notification that appears floating on top of the screen would be warranted by default, though.
Flags: needinfo?(brjones)
Flags: needinfo?(bram)
Comment 6•6 years ago
|
||
(In reply to Jan Henning [:JanH] from comment #4)
> Flod, creating a new notification channel needs a new string, so what's your
> opinion regarding fixing this on Beta?
Redirecting to Delphine, she's in charge of l10n for Fennec.
We're quickly approaching the deadline to ship any localization update in Beta (Oct 9), and IMO it's too late to introduce string changes, but it's Delphine's final call.
Flags: needinfo?(francesco.lodolo) → needinfo?(lebedel.delphine)
Comment 7•6 years ago
|
||
Thanks for flagging. Are we just talking about one new string here? When do you expect we'd get it?
As Flod says the deadline for l10n is right around the corner, so we need to evaluate if this is a must-have or not because we may end up having this string displayed in English if the turn around if not enough.
If this string is not critical and can wait another cycle, I'd vote on that (and as I understand it there's a workaround in the meantime to get sound to work). LMK
Flags: needinfo?(lebedel.delphine)
Assignee | ||
Comment 8•6 years ago
|
||
Hopefully this week, but I'm not sure. If it's too late for string uplifts then I think this could just ride the trains.
Blocks: target-android-o
Comment 9•6 years ago
|
||
Then I would say let's just have this ride the trains, IMHO
Comment 12•6 years ago
|
||
(In reply to Jan Henning [:JanH] from comment #5)
> In addition to the string changes from bug 1488874, we also need a separate
> notification channel for web notifications, since those used to be audible
> and therefore need a separate channel in order to use different default
> notification settings (also so users can configure the separately).
>
> What should that channel be called: "Site notifications", "Web
> notifications", or ...?
We can either call them “Site notifications” or “Notifications from sites”.
> Also what would sensible default settings for that channel be? Just sound,
> or sound + notification light, or even vibration? Not sure whether a
> heads-up notification that appears floating on top of the screen would be
> warranted by default, though.
When prompted with the dialogue “Do you want to receive notifications from twitter.com?”, most users who tap “Yes” expects to receive the same notification style that other apps have sent them, that they’re used to seeing. This means that we should use sound and visual, but not light. Better yet, we can default to whatever the system’s default value is.
Flags: needinfo?(bram)
Assignee | ||
Comment 13•6 years ago
|
||
Bug 1378445 made those audible and we want to preserve the behaviour.
Comment 14•6 years ago
|
||
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/b57be7d7343c
Assign site notifications to a dedicated notification channel. r=jchen
Comment 15•6 years ago
|
||
Backed out changeset b57be7d7343c (Bug 1493864) for build bustages on android-findbugs.
Backout: https://hg.mozilla.org/integration/autoland/rev/16aba5eca40ab337711f931f3b3140eba02c4c6f
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending,running,success,testfailed,busted,exception&revision=b57be7d7343c78e4484b03d98c4a1a21e03ea941
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=204914862&repo=autoland&lineNumber=4267
Flags: needinfo?(jh+bugzilla)
Comment 16•6 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Comment 17•6 years ago
|
||
Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/d1adf0333d63
Assign site notifications to a dedicated notification channel. r=jchen
Assignee | ||
Comment 18•6 years ago
|
||
Forgot the "break;" in the switch statement, sorry.
Flags: needinfo?(jh+bugzilla)
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(brjones)
Comment 19•6 years ago
|
||
bugherder |
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•