Closed Bug 1395167 Opened 7 years ago Closed 7 years ago

Page Action Menu Label Updates

Categories

(Firefox :: Menus, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.3 - Sep 19
Tracking Status
firefox57 --- fixed

People

(Reporter: abenson, Assigned: reinaldo13+bugzilla)

Details

(Whiteboard: [reserve-photon-structure] [photon-l10n-risk])

Attachments

(2 files, 3 obsolete files)

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"
Flags: qe-verify-
Priority: -- → P3
Whiteboard: [photon-structure] → [reserve-photon-structure] [photon-l10n-risk]
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.
Attached image Fix-Screenshot
Change is completed (See screenshot attached). Patch with review request will be uploaded shortly.
Attached patch page-action-menu-label-update (obsolete) — Splinter Review
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)
Assignee: nobody → reinaldo13+bugzilla
Status: NEW → ASSIGNED
Priority: P3 → P1
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)
Attached patch page-action-menu-label-update-1 (obsolete) — Splinter Review
Thanks for the feedback! Attached is the updated patch.
Attachment #8903994 - Attachment is obsolete: true
Attachment #8904414 - Flags: review?(dao+bmo)
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
https://hg.mozilla.org/mozilla-central/rev/7208ebbc9162
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
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
Iteration: --- → 57.3 - Sep 19
(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.
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)
Since this backed out should it not be re-opened ?
Yes, it should.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Iteration: 57.3 - Sep 19 → ---
Attached patch page-action-menu-label-update-2 (obsolete) — Splinter Review
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 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="&copyURLCmd.label;"
>+           copyURL-title="&copyThisLinkCmd.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)
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 on attachment 8905041 [details] [diff] [review]
page-action-menu-label-update-3.diff

Looks good, thanks again.
Attachment #8905041 - Flags: review?(dao+bmo) → review+
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
https://hg.mozilla.org/mozilla-central/rev/735938b553b5
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Iteration: --- → 57.3 - Sep 19
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]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.