Closed Bug 1368145 Opened 2 years ago Closed 2 years ago

Send to device context and page action menu should say Send to All Devices

Categories

(Firefox :: Sync, enhancement, P2)

55 Branch
enhancement

Tracking

()

VERIFIED FIXED
Firefox 56
Tracking Status
firefox56 --- verified

People

(Reporter: rfeeley, Assigned: jeongkyu.kim, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(2 files)

Attached image send-to-all-devices.png
See attached.

When we list devices that can be sent a page/link/tab from the context or new Photon page action menu, we also include a way to send to all devices.

"All Devices" is ambiguous (and has been misunderstood to mean manage devices)

We should change it to "Send to All Devices" in all menus that use it.
Mentor: eoger
Keywords: good-first-bug
Priority: -- → P2
The string is located here:
browser/chrome/browser/accounts.properties:sendTabToAllDevices.menuitem
Let me try to fix this.
Is it enough to change the property value from 'All Devices' to 'Send to All Devices'?
Nope, sadly if you do that our translators will miss the change. You have to also change the property name of the string.
I see. Sorry for my hasty review request. :-)
The what would be good for new property name?
I guess you could go for sendToAllDevices.menuitem or sendTabToAllDevices.menuitem.label, it's all about creativity here :)
The sendToAllDevices.menuitem looks good for me. This is my second day of working on Firefox, so it will take a while until my creativity comes into play. :)
Comment on attachment 8875964 [details]
Bug 1368145 - Change 'All Devices' to 'Send to All Devices' with its property name

https://reviewboard.mozilla.org/r/147370/#review151834

Awesome thanks!
Attachment #8875964 - Flags: review?(eoger) → review+
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1efa46c863a0
Change 'All Devices' to 'Send to All Devices' with its property name r=eoger
Got it. I will fix them.
By the way, is it OK to create additional commit and submit it using hg review?
You should amend your previous commit instead.
I amended the last commit but got the following error message when pushed the commit. Can you give me a clue?


$ hg push review
...
abort: Review request is submitted or discarded.
You must reopen the review request before it can be updated.
Review requests should only be reopened if your changes have not landed or have
been backed out - file new bugs for follow-up work.
You have to reopen the review request on reviewboard: https://reviewboard.mozilla.org/r/147368/
Assignee: nobody → jeongkyu.kim
Status: NEW → ASSIGNED
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 7ed69c023fb2 -d 83296fc07093: rebasing 401401:7ed69c023fb2 "Bug 1368145 - Change 'All Devices' to 'Send to All Devices' with its property name r=eoger" (tip)
merging browser/base/content/browser-sync.js
merging browser/base/content/test/general/browser_visibleTabs_contextMenu.js
merging browser/base/content/test/urlbar/browser_page_action_menu.js
merging browser/locales/en-US/chrome/browser/accounts.properties
warning: conflicts while merging browser/base/content/browser-sync.js! (edit, then use 'hg resolve --mark')
warning: conflicts while merging browser/base/content/test/general/browser_visibleTabs_contextMenu.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Sorry about this Jeongkyu, would you mind rebasing your patch?
Would you guide me on how to rebase and push the patch properly? Below is heads of my repo. 

$ hg heads .
changeset:   363449:9eb9e6b53887
tag:         tip
fxtree:      central
user:        Ben Hearsum <bhearsum@mozilla.com>
date:        Mon Jun 12 09:45:45 2017 -0400
summary:     bug 1359084: update balrog usernames in more mozharness configs. r=asasaki a=release

changeset:   363207:7ed69c023fb2
parent:      363010:f4262773c433
user:        Jeongkyu Kim <jeongkyu.kim@gmail.com>
date:        Fri Jun 09 12:25:41 2017 +0900
summary:     Bug 1368145 - Change 'All Devices' to 'Send to All Devices' with its property name r=eoger

changeset:   362752:971fba033513
user:        Jeongkyu Kim <jeongkyu.kim@gmail.com>
date:        Thu Jun 08 18:45:49 2017 +0900
summary:     Bug 1370645 - Fix sync notification message following platform conventions
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cbd4339e2ca1
Change 'All Devices' to 'Send to All Devices' with its property name. r=eoger
Sorry, this had to be backed out for failing browser-chrome test browser_contextmenu_sendpage.js:

https://hg.mozilla.org/integration/mozilla-inbound/rev/855928e7af3a0b652941edda93fc3e9660c14b5f

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=cbd4339e2ca174a730e8187a9f1c644530098709&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=106750237&repo=mozilla-inbound

[task 2017-06-13T16:21:14.849993Z] 16:21:14     INFO - Entering test bound test_page_contextmenu
[task 2017-06-13T16:21:14.852008Z] 16:21:14     INFO - TEST-PASS | browser/base/content/test/sync/browser_contextmenu_sendpage.js | checking if popup is closed - 
[task 2017-06-13T16:21:14.855669Z] 16:21:14     INFO - TEST-PASS | browser/base/content/test/sync/browser_contextmenu_sendpage.js | Send tab to device is shown - 
[task 2017-06-13T16:21:14.857727Z] 16:21:14     INFO - TEST-PASS | browser/base/content/test/sync/browser_contextmenu_sendpage.js | Send tab to device is enabled - 
[task 2017-06-13T16:21:14.863197Z] 16:21:14     INFO - TEST-PASS | browser/base/content/test/sync/browser_contextmenu_sendpage.js | Foo target is present - 
[task 2017-06-13T16:21:14.869819Z] 16:21:14     INFO - TEST-PASS | browser/base/content/test/sync/browser_contextmenu_sendpage.js | Bar target is present - 
[task 2017-06-13T16:21:14.874241Z] 16:21:14     INFO - Buffered messages finished
[task 2017-06-13T16:21:14.876348Z] 16:21:14     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/sync/browser_contextmenu_sendpage.js | All Devices target is present - Got Send to All Devices, expected All Devices
[task 2017-06-13T16:21:14.883602Z] 16:21:14     INFO - Stack trace:
[task 2017-06-13T16:21:14.886256Z] 16:21:14     INFO - chrome://mochikit/content/browser-test.js:test_is:998
[task 2017-06-13T16:21:14.891076Z] 16:21:14     INFO - chrome://mochitests/content/browser/browser/base/content/test/sync/browser_contextmenu_sendpage.js:test_page_contextmenu:23
[task 2017-06-13T16:21:14.893134Z] 16:21:14     INFO - Leaving test bound test_page_contextmenu

Please fix the issues and submit an updated patch for review.
Flags: needinfo?(jeongkyu.kim)
Pushed by eoger@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4292808c4963
Change 'All Devices' to 'Send to All Devices' with its property name. r=eoger
We got caught in a rebase storm. Fixed the patch.
Flags: needinfo?(jeongkyu.kim)
https://hg.mozilla.org/mozilla-central/rev/4292808c4963
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
I have reproduced this bug with Nightly 55.0a1 (2017-05-26) (64-bit) on Windows 7, 64 Bit!

This bug's fix is verified with latest Nightly!

Build ID : 20170614030206

User Agent : Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0


[bugday-20170614]
I have reproduced this bug with Nightly 55.0a1 (2017-05-26) (64-bit) on Ubuntu 16.04 LTS!

This bug's fix is verified with latest Nightly!

Build ID   : 20170614100332

User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170614]
As per Comment 27 and Comment 28 , I am marking this bug as Verified Fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.