Closed
Bug 1219832
Opened 9 years ago
Closed 8 years ago
Change Notification text in Preferences from "receive' to "send"
Categories
(Firefox :: Settings UI, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: tanvi, Assigned: lina)
References
Details
Attachments
(1 file, 1 obsolete file)
5.17 KB,
patch
|
MattN
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Given the early merge date, we rushed to get some changes in for the new notifications in Firefox 44. And with the rush, we got some things wrong. 1) Choose which sites are allowed to receive notifications http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/preferences/content.dtd#11 sites aren't receiving the notification, the user/browser is. 2) Control which websites are always or never allowed to receive notifications. If you remove a site, it will need to request permission again. http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/preferences/preferences.properties#28 3) Change whether the site can receive notifications http://mxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/browser.dtd#204 This will need to be landed and uplifted this week. +++ This bug was initially created as a clone of Bug #1219454 +++
Reporter | ||
Comment 1•9 years ago
|
||
Change to: 1) Choose which sites are allowed to send you notifications 2) Control which websites are always or never allowed to send you notifications. If you remove a site, it will need to request permission again. 3) Change whether the site can send you notifications
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → kcambridge
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8680870 -
Flags: review?(MattN+bmo)
Comment 3•9 years ago
|
||
Comment on attachment 8680870 [details] [diff] [review] 1219832.patch Review of attachment 8680870 [details] [diff] [review]: ----------------------------------------------------------------- Don't you need to also change the initial doorhanger string too? urlbar.webNotsNotificationAnchor3.label should match it since they're used together. webNotifications.receiveFromSite=Would you like to receive notifications from this site?
Attachment #8680870 -
Flags: review?(MattN+bmo) → feedback+
Assignee | ||
Comment 4•9 years ago
|
||
Ah, I asked about that on IRC...
> 11:47 <kitcambridge> tanvi: should we change the doorhanger prompt, too? "Would you like to receive notifications from this site?" -> "Would you like this site to send you notifications?"
> 11:47 <tanvi> kitcambridge: i think we should leave that as is for now
> 11:48 <kitcambridge> cool, will do. thanks!
> 11:48 <tanvi> that is what we got from talking to bmaggs, phlsa, etc. and we were tryign to capture the fact that the messages might be received int eh background
> 11:48 <tanvi> its not a bad idea, but i think at this point we shoudl leave it
> 11:48 <kitcambridge> makes sense
> 11:50 <edwong> yeah - no more string changes ;)
Are you OK with changing the doorhanger text, Tanvi?
Flags: needinfo?(tanvi)
Comment 5•9 years ago
|
||
The main thing is that we need the two strings mentioned in comment 3 to be consistent as they're both for the doorhanger permission prompt.
Reporter | ||
Comment 6•9 years ago
|
||
At this point, I think we should leave urlbar.webNotsNotificationAnchor2.label as it is then. To change the main permission string (the only one that most users are going to see), we should talk to Matej and Philipp too.
Flags: needinfo?(tanvi)
Comment 7•9 years ago
|
||
I agree with Tanvi. We should get input from UX on this important permission. It can stay for now.
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8680870 -
Attachment is obsolete: true
Attachment #8681008 -
Flags: review?(MattN+bmo)
Comment 9•9 years ago
|
||
(In reply to Bill Maggs (bmaggs) from comment #7) > I agree with Tanvi. We should get input from UX on this important > permission. No rationale was given for why the 2 related strings shouldn't be unified. > It can stay for now. I don't want to uplift another bug so we should sort it out once and for all now and this needs to land ASAP.
Flags: needinfo?(wmaggs)
Updated•9 years ago
|
Attachment #8681008 -
Flags: review?(MattN+bmo) → review+
Comment 11•9 years ago
|
||
Comment on attachment 8681008 [details] [diff] [review] 1219832.patch Approval Request Comment [Feature/regressing bug #]: Notifications [User impact if declined]: Confusing string about where the notifications is coming from and/or going to. [Describe test coverage new/current, TreeHerder]: N/A string change [Risks and why]: Low risk string change which would very obviously fail if done wrong [String/UUID change made/needed]: yes, string-change for clarity
Attachment #8681008 -
Flags: approval-mozilla-aurora?
Comment 12•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/6d6bcbe0f1ae
status-firefox44:
--- → fixed
Comment 13•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e3afc3186d85
Comment 14•9 years ago
|
||
Don't know why this had leave-open… probably from cloning.
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: needinfo?(wmaggs)
Keywords: leave-open
Resolution: --- → FIXED
Updated•9 years ago
|
Component: DOM: Push Notifications → Preferences
Product: Core → Firefox
Comment on attachment 8681008 [details] [diff] [review] 1219832.patch This one had a blanket approval.
Attachment #8681008 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 16•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e3afc3186d85
status-b2g-v2.5:
--- → fixed
Comment 17•9 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/6d6bcbe0f1ae
status-b2g-v2.5:
fixed → ---
Comment 18•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/file/f8b569906e4c/browser/locales/en-US/chrome/browser/sitePermissions.properties#l11
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 19•8 years ago
|
||
(In reply to Stefan Plewako [:stef] from comment #18) > https://hg.mozilla.org/mozilla-central/file/f8b569906e4c/browser/locales/en- > US/chrome/browser/sitePermissions.properties#l11 Putting a link without any explanation is not really useful. As explained in the past, please file new bugs if you find issues, reopening existing bugs, especially with this much tracking and landing, doesn't help.
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
Resolution: --- → FIXED
Comment 20•8 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #19) > Putting a link without any explanation is not really useful. Sorry for reopening this one. Filling new bugs and commenting in existing doesn't seem useful anymore too. With given link anyone interested will be able to spot the problem and fix it.
You need to log in
before you can comment on or make changes to this bug.
Description
•