Closed Bug 1225519 Opened 4 years ago Closed Last year

Permission in Page info should say "Send notifications" instead of "Receive notifications"

Categories

(Firefox :: Page Info Window, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 64
Tracking Status
firefox64 --- fixed

People

(Reporter: flod, Assigned: manishkk, Mentored)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

Status: RESOLVED → REOPENED
Resolution: INACTIVE → ---
Mentor: jhofmann
Status: REOPENED → NEW
Keywords: good-first-bug
Priority: -- → P3
Assignee: nobody → 1991manish.kumar
Attached patch Patch_Bug1225519 (obsolete) — Splinter Review
Please review.
Flags: needinfo?(jhofmann)
Needinfo is to ask for information. In this case, you're attaching a patch, so you can set the review flag to Johann.

Having said that: you're changing an existing string, which means you need to use a new ID in the .properties file, and update the code to reference it (potentially look for tests too).
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings
Flags: needinfo?(jhofmann)
How can I test this code?

'./mach mochitest'
Flags: needinfo?(jhofmann)
(In reply to Manish Kumar [:manishkk] from comment #4)
> How can I test this code?
> 
> './mach mochitest'

Since you're only updating copy I'm not sure you really need to test it. If you'd like to test a bit of UI that is affected by this locally, you can run

./mach mochitest browser/base/content/test/permissions

or

./mach mochitest browser/base/content/test/pageinfo

You can also do a try run (https://wiki.mozilla.org/ReleaseEngineering/TryServer). Do you have try access? If not, I'm happy to vouch for you.

I'd personally recommend just going to https://permission.site/, accepting the notification prompt and checking the updated label in the identity popup.
Flags: needinfo?(jhofmann)
Thanks, you already vouch for me.
@manish
Were you able to make any progress with this bug?
Flags: needinfo?(1991manish.kumar)
:johannh

After the changes: 

This fails this test:

./mach mochitest browser/base/content/test/permissions

https://pastebin.mozilla.org/9092694
Flags: needinfo?(1991manish.kumar) → needinfo?(francesco.lodolo)
Hi Manish! Did you update https://searchfox.org/mozilla-central/rev/c3fef66a5b211ea8038c1c132706d02db408093a/browser/modules/SitePermissions.jsm#757 with the new string ID, too?
Flags: needinfo?(francesco.lodolo) → needinfo?(1991manish.kumar)
Attached patch Patch_Bug1225519Splinter Review
Yes! I updated string ID also.

Please check patch!
Attachment #8986942 - Attachment is obsolete: true
Flags: needinfo?(1991manish.kumar)
Attachment #9006098 - Flags: review?(lina)
Comment on attachment 9006098 [details] [diff] [review]
Patch_Bug1225519

LGTM, but I've triggered a Try run just in case. :-) Thanks, Manish! https://treeherder.mozilla.org/#/jobs?repo=try&revision=e32357def06b6ddb7f45d46006a06a503c69f020
Attachment #9006098 - Flags: review?(lina) → review+
Keywords: checkin-needed
Pushed by dluca@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/10da2f00e15d
Permission in Page info should say Send notifications instead of Receive notifications r=lina
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/10da2f00e15d
Status: NEW → RESOLVED
Closed: 2 years agoLast year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
I have reproduced this bug with Nightly 45.0a1 (2015-11-18) on Windows 10, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID   - 20180910220142
User Agent - Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64
QA Whiteboard: [bugday-20180905]
You need to log in before you can comment on or make changes to this bug.