Closed
Bug 1395167
Opened 7 years ago
Closed 7 years ago
Page Action Menu Label Updates
Categories
(Firefox :: Menus, enhancement, P1)
Firefox
Menus
Tracking
()
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: abenson, Assigned: reinaldo13+bugzilla)
Details
(Whiteboard: [reserve-photon-structure] [photon-l10n-risk])
Attachments
(2 files, 3 obsolete files)
36.58 KB,
image/png
|
Details | |
3.28 KB,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
Update labels for more consistency across list items: 1. Change "Copy URL" to "Copy Link" (Make sure the tooltip matches when it's pinned to the URL bar) 2. Change "Send Page to Device" to "Send Tab to Device"
Updated•7 years ago
|
Flags: qe-verify-
Priority: -- → P3
Whiteboard: [photon-structure] → [reserve-photon-structure] [photon-l10n-risk]
Assignee | ||
Comment 1•7 years ago
|
||
First time contributor (to be) here. I can take a look, and work on this one if no one else is currently working on it. I should be able to submit a patch within the next day or two.
Assignee | ||
Comment 2•7 years ago
|
||
Change is completed (See screenshot attached). Patch with review request will be uploaded shortly.
Assignee | ||
Comment 3•7 years ago
|
||
Here is the patch. Please review when you get a chance and let me know if there is any feedback or suggestions.
Attachment #8903994 -
Flags: review?(dao+bmo)
Updated•7 years ago
|
Assignee: nobody → reinaldo13+bugzilla
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment 4•7 years ago
|
||
Comment on attachment 8903994 [details] [diff] [review] page-action-menu-label-update Thanks for the patch! Please update the entity names of these strings (e.g. copyLinkCmd.label, sendPageToDevice.label), otherwise the other locales besides US English won't pick up this change.
Attachment #8903994 -
Flags: review?(dao+bmo)
Assignee | ||
Comment 5•7 years ago
|
||
Thanks for the feedback! Attached is the updated patch.
Attachment #8903994 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8904414 -
Flags: review?(dao+bmo)
Comment 6•7 years ago
|
||
Comment on attachment 8904414 [details] [diff] [review] page-action-menu-label-update-1 Perfect, thanks!
Attachment #8904414 -
Flags: review?(dao+bmo) → review+
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7208ebbc9162 Relabel "Copy URL" to "Copy Link" and "Send Page to Device" to "Send Tab to Device" in the page action menu. r=dao
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7208ebbc9162
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment 9•7 years ago
|
||
Note that this change adds 2 already existing entitity names and causes issues. They should probably be unique. https://l10n.mozilla.org/dashboard/compare?run=795056
Updated•7 years ago
|
Iteration: --- → 57.3 - Sep 19
Comment 10•7 years ago
|
||
(In reply to Ton from comment #9) > Note that this change adds 2 already existing entitity names and causes > issues. They should probably be unique. Without "probably". Not sure if it keeps the first or the last occurrence of the string, but now one of the two elements have the wrong label. <!ENTITY copyLinkCmd.label "Copy Link"> <!ENTITY copyLinkCmd.label "Copy Link Location"> The other string is identical, but I still suggest to use a different ID since the context is different <!ENTITY sendTabToDevice.label "Send Tab to Device"> <!ENTITY sendTabToDevice.label "Send Tab to Device"> I hate to backout a bug from a first time contributor, but the alternative is to land a fix very quickly.
Comment 11•7 years ago
|
||
Sorry, this had to be backed out for duplicate string entity ids: https://hg.mozilla.org/mozilla-central/rev/b235fb79d6e017b9f47309cb06eb701c06b7e8d2 From the log linked in comment 9: copyLinkCmd.label occurs 2 times sendTabToDevice.label occurs 2 times You need unique names which aren't used in browser.dtd yet. Please fix the issue and submit an updated patch. Thank you.
Flags: needinfo?(reinaldo13+bugzilla)
Comment 12•7 years ago
|
||
Since this backed out should it not be re-opened ?
Updated•7 years ago
|
Iteration: 57.3 - Sep 19 → ---
Assignee | ||
Comment 14•7 years ago
|
||
No worries about the back-out. I'm glad that you guys caught it. Attached is the new patch. Thanks again.
Attachment #8904414 -
Attachment is obsolete: true
Flags: needinfo?(reinaldo13+bugzilla)
Attachment #8905019 -
Flags: review?(dao+bmo)
Attachment #8905019 -
Flags: review?(aryx.bugmail)
Comment 15•7 years ago
|
||
Comment on attachment 8905019 [details] [diff] [review] page-action-menu-label-update-2 >diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul >--- a/browser/base/content/browser.xul >+++ b/browser/base/content/browser.xul >@@ -432,19 +432,19 @@ > role="group" > type="arrow" > hidden="true" > flip="slide" > photon="true" > position="bottomcenter topright" > tabspecific="true" > noautofocus="true" >- copyURL-title="©URLCmd.label;" >+ copyURL-title="©ThisLinkCmd.label;" > emailLink-title="&emailPageCmd.label;" >- sendToDevice-title="&sendToDevice.label3;" >+ sendToDevice-title="&sendThisTabToDevice.label;" I think we should probably reuse the pageAction "namespace" here, i.e.: &pageAction.copyLink.label; &pageAction.sendTabToDevice.label;
Attachment #8905019 -
Flags: review?(dao+bmo)
Attachment #8905019 -
Flags: review?(aryx.bugmail)
Assignee | ||
Comment 16•7 years ago
|
||
Applied latest review feedback (adding pageAction namespace). Here (attached) is the patch.
Attachment #8905019 -
Attachment is obsolete: true
Attachment #8905041 -
Flags: review?(dao+bmo)
Comment 17•7 years ago
|
||
Comment on attachment 8905041 [details] [diff] [review] page-action-menu-label-update-3.diff Looks good, thanks again.
Attachment #8905041 -
Flags: review?(dao+bmo) → review+
Comment 18•7 years ago
|
||
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/735938b553b5 Relabel "Copy URL" to "Copy Link" and "Send Page to Device" to "Send Tab to Device" in the page action menu. r=dao
Comment 19•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/735938b553b5
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Iteration: --- → 57.3 - Sep 19
Comment 20•7 years ago
|
||
I have reproduced this bug with Nightly 57.0a1 (2017-08-30) on Windows 8.1, 64 Bit! This bug's fix is verified on Latest Nightly 57.0a1. Build ID : 20170910100150 User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20170906]
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•