1363182 - Add a "send to device" subview to the page action menu

Status: VERIFIED FIXED

(Firefox :: Address Bar, enhancement, P1)

Description

[:Gijs (he/him)]

See https://mozilla.invisionapp.com/share/ENBBK0F9U#/screens/232444774 for a spec (from bug 1360055). This should list all devices and provide an 'all devices' option, much like the current context menu on tabs.

Comment 1

[Drew Willcoxon :adw]

Bryan, Aaron, what should happen in these two special cases?

(1) The current page isn't sendable.  That's possible in some cases, including when the URL is too long, or it's a local file URL, among others.  Full logic here if it helps: https://dxr.mozilla.org/mozilla-central/rev/b8e9b674033bcd1f3a4c59b9d0ee7619c1a17cc5/browser/base/content/browser-sync.js#333

(2) You don't have any remote devices.  (In Firefox today, we just don't show the send-to-devices menu item and submenu at all.)

Comment 2

[Drew Willcoxon :adw]

Attachment: Bug 1363182 - Add a "send to device" subview to the page action menu.

Review commit: https://reviewboard.mozilla.org/r/139914/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/139914/

Comment 3

[Drew Willcoxon :adw]

WIP that's complete except for answers to comment 1.

Comment 4

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #1)
> (1) The current page isn't sendable.  That's possible in some cases,
> including when the URL is too long, or it's a local file URL, among others. 

Or if you don't have an account or aren't signed in!

Comment 5

[bbell]

Seems like this is a related Bug #1359894 which we do not have to implement for Photon, just that other have been thinking about it.

Comment 6

[bbell]

here's the simplest thing I can think to do:  https://mozilla.invisionapp.com/share/KZBRDI1EU#/screens/234257205_Action_Menu_-_Send_To_Device


Let's just link people to the sync preference if they are not logged in.

Comment 7

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139914/diff/1-2/

Comment 8

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Mark, could you please review these browser-sync.js changes or redirect to someone who could?  We're adding a "Send to Devices" submenu to this new Photon page action panel.  This patch generalizes populateSendTabToDevicesMenu so that it can be used to populate any container with any type of items.  In my case, I need to populate a vbox with toolbarbuttons.

Also, my panel has a single toolbarbutton already in it that links to the Fxa preferences pane.  That button is hidden if there are any devices.  That's why I'm adding a new "sync-menuitem" class to the items that populateSendTabToDevicesMenu creates: so that it can tell which items it has created and should therefore be removed.

Comment 9

[Drew Willcoxon :adw]

Mike, I asked Mark to look at the browser-sync.js part (obviously, my previous comment), but feel free to look at that too if you'd like of course.

Comment 10

[Drew Willcoxon :adw]

Mark, also, I'm adding a clientType attribute to these items so that I can style mobile devices differently from desktops.  (Mobile gets a phone icon, desktop gets a desktop icon, via CSS.)

Comment 11

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

This looks fine to me with a quick glance. Edouard, do you mind doing the formal review?

Comment 12

[Mark Hammond [:markh] [:mhammond]]

(oh, and I meant to say "woo hoo" :)

Comment 13

[Mike de Boer [:mikedeboer]]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

https://reviewboard.mozilla.org/r/139914/#review144518

Alright, this looks really good Drew! \o/ Thanks!

As a more general comment: have you thought about adding browser tests for the new action menu? Would it be a good idea to add to a) this bug or b) create a follow-up to create one which'll cover the current implementation?

I'd like to see this patch one more time after your updates.

::: commit-message-9c0a9:1
(Diff revision 2)
> +Bug 1363182 - Add a "send to device" subview to the page action menu. r?mikedeboer

Can you add a more detailed message below the commit title that summarizes what you needed to change to make this all work?

::: browser/base/content/browser-sync.js:293
(Diff revision 2)
>    sendTabToDevice(url, clientId, title) {
>      Weave.Service.clientsEngine.sendURIToClientForDisplay(url, clientId, title);
>    },
>  
> -  populateSendTabToDevicesMenu(devicesPopup, url, title) {
> +  populateSendTabToDevicesMenu(popup, url, title) {
> +    this.populateSendTabToDevicesMenuEx(popup, url, title, (clientId, name, clientType) => {

Ex? Usually I prefer the following:

```js
popuplateSendTabToDevicesMenu(popup, url, title, createDeviceNodeFn) {
  if (!createDeviceNodeFn) {
    createDeviceNodeFn = (clientId, name, clientType) => {
      let eltName = name ? "menuitem" : "menuseparator";
      return document.createElement(eltName);
    }
  }
  //...
}
```

::: browser/base/content/browser-sync.js:301
(Diff revision 2)
> +    });
> +  },
> +
> +  populateSendTabToDevicesMenuEx(devicesPopup, url, title, createDeviceNodeFn) {
>      // remove existing menu items
> -    while (devicesPopup.hasChildNodes()) {
> +    for (let i = 0; i < devicesPopup.childNodes.length; ) {

nit: didn't ESLint throw up here? Superfluous space.

::: browser/base/content/browser-sync.js:301
(Diff revision 2)
> +    });
> +  },
> +
> +  populateSendTabToDevicesMenuEx(devicesPopup, url, title, createDeviceNodeFn) {
>      // remove existing menu items
> -    while (devicesPopup.hasChildNodes()) {
> +    for (let i = 0; i < devicesPopup.childNodes.length; ) {

Why not iterate backwards? That way you can write this like:

```js
for (let i = devicePopup.childNodes.length - 1; i >= 0; --i) {
  let child = devicePopup.childNode[i];
  if (!child.classList.contains("sync-menuitem") {
    continue;
  }
  child.remove();
}
```

(Just sayin', as you might find it easier to read as well.)

::: browser/base/content/browser-sync.js:323
(Diff revision 2)
>      }
>  
> -    function addTargetDevice(clientId, name) {
> -      const targetDevice = document.createElement("menuitem");
> +    function addTargetDevice(clientId, name, clientType) {
> +      const targetDevice = createDeviceNodeFn(clientId, name, clientType);
>        targetDevice.addEventListener("command", onTargetDeviceCommand, true);
> -      targetDevice.setAttribute("class", "sendtab-target");
> +      targetDevice.classList.add("sync-menuitem");

You can pass multiple classNames to .add():

```js
targetDevice.classList.add("sync-menuitem", "sendtab-target");
```

::: browser/base/content/browser.js:7935
(Diff revision 2)
> +    gSync.populateSendTabToDevicesMenuEx(body, url, title, (clientId, name, clientType) => {
> +      if (!name) {
> +        return document.createElement("toolbarseparator");
> +      }
> +      let item = document.createElement("toolbarbutton");
> +      item.classList.add("page-action-sendToDevice-device");

Same here, wrt passing multiple args to `add()`

::: browser/base/content/browser.xul:474
(Diff revision 2)
> +        </panelview>
> +        <panelview id="page-action-sendToDeviceView"
> +                   class="PanelUI-subView"
> +                   title="&sendToDevice.viewTitle;">
> +          <vbox id="page-action-sendToDeviceView-body"
> +                class="panel-subview-body">

nit: You can keep these on one line, I think.

::: browser/themes/shared/browser.inc.css:99
(Diff revision 2)
>    fill: currentColor;
>  }
>  
>  #page-action-email-link-button {
>    list-style-image: url("chrome://browser/skin/email-link.svg");
>    -moz-context-properties: fill;

Hmm, perhaps we should just declare these on subviewbuttons more generally? I mean '-moz-context-properties: fill' and 'fill: currentColor'.

::: browser/themes/shared/icons/device-desktop.svg:1
(Diff revision 2)
> +<?xml version="1.0"?>

Please drop the processing intruction in SVG files.

Comment 14

[Edouard Oger [:eoger]]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

https://reviewboard.mozilla.org/r/139914/#review144704

Looks great!
Please also be aware that in bug 1359894 we'll be adding a better special-case for non-configured/unverified sync users, but your pref button if fine for now.

::: browser/base/content/browser-sync.js:299
(Diff revision 2)
> +      let eltName = name ? "menuitem" : "menuseparator";
> +      return document.createElement(eltName);
> +    });
> +  },
> +
> +  populateSendTabToDevicesMenuEx(devicesPopup, url, title, createDeviceNodeFn) {

Just call this _populateSendTabToDevicesMenu

::: browser/base/content/browser-sync.js:324
(Diff revision 2)
>  
> -    function addTargetDevice(clientId, name) {
> -      const targetDevice = document.createElement("menuitem");
> +    function addTargetDevice(clientId, name, clientType) {
> +      const targetDevice = createDeviceNodeFn(clientId, name, clientType);
>        targetDevice.addEventListener("command", onTargetDeviceCommand, true);
> -      targetDevice.setAttribute("class", "sendtab-target");
> +      targetDevice.classList.add("sync-menuitem");
> +      targetDevice.classList.add("sendtab-target");

It's too bad we're using 2 classes (sendtab-target and sync-menuitem) here, but I don't really care

Comment 15

[Drew Willcoxon :adw]

(In reply to Mike de Boer [:mikedeboer] from comment #13)
> As a more general comment: have you thought about adding browser tests for
> the new action menu? Would it be a good idea to add to a) this bug or b)
> create a follow-up to create one which'll cover the current implementation?

Yeah, it would be good to have tests.  I should probably include them in the patch for this bug.

> ::: browser/themes/shared/browser.inc.css:99
> (Diff revision 2)
> >    fill: currentColor;
> >  }
> >  
> >  #page-action-email-link-button {
> >    list-style-image: url("chrome://browser/skin/email-link.svg");
> >    -moz-context-properties: fill;
> 
> Hmm, perhaps we should just declare these on subviewbuttons more generally?
> I mean '-moz-context-properties: fill' and 'fill: currentColor'.

That's a good idea, or at least add a single selector that includes them on all these buttons in this panel.

Comment 16

[Drew Willcoxon :adw]

Thanks Edouard.

(In reply to Edouard Oger [:eoger] from comment #14)
> ::: browser/base/content/browser-sync.js:299
> (Diff revision 2)
> > +      let eltName = name ? "menuitem" : "menuseparator";
> > +      return document.createElement(eltName);
> > +    });
> > +  },
> > +
> > +  populateSendTabToDevicesMenuEx(devicesPopup, url, title, createDeviceNodeFn) {
> 
> Just call this _populateSendTabToDevicesMenu

I think I'll do what Mike suggested in comment 13: a single function that takes an optional createDeviceNodeFn.  If it's not passed in, then use the default that creates menuitems.

Also IMO avoiding an underscore would be better, since underscores usually imply that the method is "private," but here I need to call it from a consumer.

> ::: browser/base/content/browser-sync.js:324
> (Diff revision 2)
> >  
> > -    function addTargetDevice(clientId, name) {
> > -      const targetDevice = document.createElement("menuitem");
> > +    function addTargetDevice(clientId, name, clientType) {
> > +      const targetDevice = createDeviceNodeFn(clientId, name, clientType);
> >        targetDevice.addEventListener("command", onTargetDeviceCommand, true);
> > -      targetDevice.setAttribute("class", "sendtab-target");
> > +      targetDevice.classList.add("sync-menuitem");
> > +      targetDevice.classList.add("sendtab-target");
> 
> It's too bad we're using 2 classes (sendtab-target and sync-menuitem) here,
> but I don't really care

Yeah, I almost went with using sendtab-target, but I wasn't sure how that's used, and also menuseparators don't get that class at all, so I thought it was safer to add a new class.

Comment 17

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139914/diff/2-3/

Comment 18

[Drew Willcoxon :adw]

Updated for requested changes.  I'm working on a test, and when I finish that, I'll post another patch and ask for review.

Comment 19

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139914/diff/3-4/

Comment 20

[Drew Willcoxon :adw]

Complete patch with test for all page action panel functionality (so far).

Comment 21

[Mike de Boer [:mikedeboer]]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

https://reviewboard.mozilla.org/r/139914/#review145096

What I find quite confusing is that the button 'Firefox Account    required' is always visible, also when I'm signed in. What I expect to see, when I'm signed in, is:
a) a list of my connected devices, when I have one or more
b) a (whimsical) message, saying that there are no devices available to send stuff to, but will show up when I sign in there. Perhaps even incentivise me to use Fx on mobile.

This is partly a UX question, of course, but I think important enough to withhold landing this just yet. Depending on Bryan's or Aaron's answer, this might become follow-up fodder. If that's the case, r=me.

::: commit-message-9c0a9:3
(Diff revision 4)
> +Bug 1363182 - Add a "send to device" subview to the page action menu. r?mikedeboer
> +
> +Add a Send to Device subview to the page action panel.  When the page isn't sendable, disable the Send to Device menu item.  When the user doesn't have any devices, show a menu item that opens the Firefox Account preferences pane.

I generally try to adhere to an 80 column rule here and recommend it here too.

::: browser/base/content/browser.js:7925
(Diff revision 4)
> +    this.populateSendToDeviceView();
> +    PanelUI.showSubView("page-action-sendToDeviceView", subviewButton);
> +  },
> +
> +  populateSendToDeviceView() {
> +    let browser = gBrowser.selectedBrowser;

Since you're not using this function anywhere else, can you inline it into `showSendToDeviceView`?

::: browser/base/content/test/urlbar/browser_page_action_menu.js:6
(Diff revision 4)
> +"use strict";
> +
> +let gPanel = document.getElementById("page-action-panel");
> +
> +add_task(async function copyURL() {
> +  // Open the panel.

I'm surprised you don't need to set the photon-structure pref... how does this work?

Comment 22

[Mike de Boer [:mikedeboer]]

Aaron or Bryan, please see the first two paragraphs in comment 13 for my question. Thanks!

Comment 23

[:Gijs (he/him)]

(In reply to Mike de Boer [:mikedeboer] from comment #21)
> ::: browser/base/content/test/urlbar/browser_page_action_menu.js:6
> (Diff revision 4)
> > +"use strict";
> > +
> > +let gPanel = document.getElementById("page-action-panel");
> > +
> > +add_task(async function copyURL() {
> > +  // Open the panel.
> 
> I'm surprised you don't need to set the photon-structure pref... how does
> this work?

Dão changed the button (and thereby panel) to live behind the PHOTON_THEME ifdef instead because it suited the styling aims for the location bar better.

That said, this would mean the test will start failing on aurora. :-\

Comment 24

[Mike de Boer [:mikedeboer]]

OK, then we better guard the test using PHOTON_THEME in browser.ini as well.

Comment 25

[bbell]

(In reply to Mike de Boer [:mikedeboer] from comment #22)
> Aaron or Bryan, please see the first two paragraphs in comment 13 for my
> question. Thanks!

I'm sorry but I'm not sure what the question is.

Comment 26

[Mike de Boer [:mikedeboer]]

I meant comment 21. I had a talk with Aaron today and he'll discuss it with you later when you're available after travels.

Comment 27

[Drew Willcoxon :adw]

(In reply to Mike de Boer [:mikedeboer] from comment #21)
> What I find quite confusing is that the button 'Firefox Account    required'
> is always visible, also when I'm signed in.

Based on the spec, that item should be visible "if a user doesn't have any synced devices or a Firefox account."  Just want to make sure the implementation is working as expected -- if you are seeing that item, then you don't have any synced devices, correct?

I'll leave the question of whether to show that item when you're logged in to Bryan.

Comment 28

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139914/diff/4-5/

Comment 29

[Drew Willcoxon :adw]

This addresses the comments in comment 21 except for the question about the Fxa button.  I'll clear the review flag until that's addressed.

(In reply to Mike de Boer [:mikedeboer] from comment #24)
> OK, then we better guard the test using PHOTON_THEME in browser.ini as well.

I don't know how to do that.  It doesn't look like ini files can be preprocessed, and I don't think we have a skip-if for Photon...?  So instead I just did:

> run-if = nightly_build # Photon only

Comment 30

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139914/diff/5-6/

Comment 31

[Drew Willcoxon :adw]

Couple of fixes:

(1) eslint

(2) browser/base/content/test/general/contextmenu_common.js was comparing node.className to an expected class, which this patch breaks since it adds a new "sync-menuitem" class to device nodes.  I changed that comparison to use classList.contains.

Comment 32

[Mike de Boer [:mikedeboer]]

Leaving the n-i request for Bryan. I talked with Aaron today that he'd explain the situation to Bryan what needs to be decided/ designed still.

I'll take another look at the patch tomorrow of course, but please feel free to work on other things in the meantime :)

Comment 33

[Aaron Benson]

Updated the spec to include a clearer message for when a sync account is logged in but there are no devices to sync to: https://mozilla.invisionapp.com/share/KZBRDI1EU#/screens/234257205_Action_Menu_-_Send_To_Device

Comment 34

[Mike de Boer [:mikedeboer]]

(In reply to Drew Willcoxon :adw from comment #29)
> > run-if = nightly_build # Photon only

That looks a-ok to me!

Comment 35

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139914/diff/6-7/

Comment 36

[Drew Willcoxon :adw]

Mike, here's a patch with a messy narrowed-down test.  Please ignore all the commented-out code.  It has two uncommented-out task functions.  Both of them open the panel, try to click the devices item, and check the items that appear in the devices subview.  There are other uncommented-out helper functions at the bottom.

In the first task, the subview should contain a single "No Devices" item.  In the second task, it should contain four device items, a separator, and an "All Devices" item.

In the first task, sometimes the panel shrinks to like 20 points tall when the devices item is clicked.  The task finishes successfully, I guess because the subview must be shown correctly, but it's hard to tell.  But then when the second task starts and opens the panel again, the panel is still 20 points tall, so the task can't click the devices item, and it hangs waiting for the subview to show.

It shouldn't take you many tries to see that happen.  For me, it seems like 2 out of 3 runs or so.

Sometimes that doesn't happen though, and both tasks finish successfully.  But even then, when the second task shows the subview, the panel doesn't expand to a sufficient height to show all the items in the subview.

Comment 37

[Mike de Boer [:mikedeboer]]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

https://reviewboard.mozilla.org/r/139914/#review145698

Nice work. I hope to be able to sort out the test wonkyness quickly tomorrow!

I have a few small things and cleaning/ annotating the test would be most helpful.

::: browser/base/content/browser.css:1323
(Diff revision 7)
> +#page-action-sendToDeviceView-body[signedin] > #page-action-sendToDevice-fxa-button {
> +  display: none;
> +}
> +
> +#page-action-sendToDeviceView-body:not([signedin]) > #page-action-no-devices-button,
> +#page-action-sendToDeviceView-body[hasdevices] > #page-action-no-devices-button {

Can you merge these two together?

::: browser/base/content/browser.xul:477
(Diff revision 7)
> +                   title="&sendToDevice.viewTitle;">
> +          <vbox id="page-action-sendToDeviceView-body" class="panel-subview-body">
> +            <toolbarbutton id="page-action-sendToDevice-fxa-button"
> +                           class="subviewbutton subviewbutton-iconic"
> +                           label="&syncBrand.fxAccount.label;"
> +                           shortcut="&sendToDevice.fxaRequired.label;"

Not blocking on this, but what are you using `shortcut` here for?

::: browser/base/content/test/general/contextmenu_common.js:52
(Diff revision 7)
>  
>          var isPageMenuItem = item.hasAttribute("generateditemid");
>  
>          if (item.nodeName == "menuitem") {
> -            var isGenerated = item.className == "spell-suggestion"
> -                              || item.className == "sendtab-target";
> +            var isGenerated = item.classList.contains("spell-suggestion")
> +                              || item.classList.contains("sendtab-target");

Good call!

::: browser/base/content/test/urlbar/browser_page_action_menu.js:5
(Diff revision 7)
> +"use strict";
> +
> +let gPanel = document.getElementById("page-action-panel");
> +
> +// add_task(async function copyURL() {

I see you tried many things!

In order for me to look at this properly, can you 

1. Clean it up a little bit so that all the things you _know_ I don't need are not in here,
2. annotate this file with comments with the `add_task()`s that need to be enabled to land this patch.

That'd help much with the investigation. I'll be writing the other panel test as well, so it'll be a useful excercise!

Thanks!

Comment 38

[Drew Willcoxon :adw]

(In reply to Mike de Boer [:mikedeboer] from comment #37)
> ::: browser/base/content/browser.xul:477
> (Diff revision 7)
> > +                   title="&sendToDevice.viewTitle;">
> > +          <vbox id="page-action-sendToDeviceView-body" class="panel-subview-body">
> > +            <toolbarbutton id="page-action-sendToDevice-fxa-button"
> > +                           class="subviewbutton subviewbutton-iconic"
> > +                           label="&syncBrand.fxAccount.label;"
> > +                           shortcut="&sendToDevice.fxaRequired.label;"
> 
> Not blocking on this, but what are you using `shortcut` here for?

The "Required" text on the right edge of the button.  The spec shows that looking just like shortcuts do, so I used that here.  It looks like `shortcut` is only a visual thing, right, and not actually hooked up to any key handling?  https://mozilla.invisionapp.com/share/KZBRDI1EU#/screens/234257205_Action_Menu_-_Send_To_Device

> 1. Clean it up a little bit so that all the things you _know_ I don't need
> are not in here,

Yeah sorry for all the commented-out code.  That's all the actual test code that's not related to debugging this problem.  Everything that's not commented out is the smallest I could get it down to.

It would probably be possible to whittle it down even more if I took the problem out of the context of this bug, and just made a dummy panel with dummy items.  Would you like me to do that?  But that would only avoid the Sync-related parts of the test, which aren't substantial (promiseSyncReady, UIState, gSync).

> 2. annotate this file with comments with the `add_task()`s that need to be
> enabled to land this patch.

Not sure what you mean.  Were you thinking you would land this for me?  I was thinking you'd use it as a starting point or helper for debugging the panel problem and then you'd let me know whether it's a PanelUI problem or something I'm doing wrong.  Then I can make any necessary fixes to it and land it.

Comment 39

[Mike de Boer [:mikedeboer]]

I think I've got good new here: my changes in bug 1365647 seem to have fixed the issue you were seeing. Can you check that?

Comment 40

[Drew Willcoxon :adw]

Thanks Mike.  That bug (or something else that landed recently) seems to have fixed half the problem I was seeing.  It fixes the part where the panel is only 20 pts tall.  It doesn't fix the part where the second time the panel opens, its height is insufficient to show the entire contents of the panel.

But, my test now passes because it doesn't do anything with the part of the panel that's cut off.  If it did, there would still be a problem here.

Comment 41

[Drew Willcoxon :adw]

Comment on attachment 8868332 [details]
Bug 1363182 - Add a "send to device" subview to the page action menu.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/139914/diff/7-8/

Comment 42

[Drew Willcoxon :adw]

Rebased on current tree, addressed Mike's comments.  I'll do one more try push and then land this if it looks OK.

Comment 43

[Drew Willcoxon :adw]

Try looks OK, landing.  Looks like I might have just missed the merge to m-c today though.

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15688bfe56c9
Add a "send to device" subview to the page action menu. r=eoger,mikedeboer

Comment 45

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/15688bfe56c9

Comment 46

[mdew]

Just a question, If the user doesn't anything to "send to", does the menu show up?

Comment 47

[Drew Willcoxon :adw]

If you're signed in but don't have any devices, then the menu will show up, and the submenu will say, "No Devices."  If you're not signed in, then the menu will also show up, and the submenu will say, "Firefox Account Required."

Comment 48

[Sayed [:LiquidO2]]

I can see "send to device" subview in page actions drop down menu in latest Nightly 55.0a1 on Deepin OS, 64bit.

But when selecting any device, it shows no visual confirmation that it has been sent to the device. Some kind of confirmation would be great.

Build ID 	20170531100318
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170531]

Comment 49

[Lina Butler [:lina]]

(In reply to Sayed from comment #48)
> But when selecting any device, it shows no visual confirmation that it has
> been sent to the device. Some kind of confirmation would be great.

Check out bug 1368384. :-)

Comment 50

[Azmina]

I have seen the feature "send to device" subview to the page action menu has been implemented on Latest Nightly in Windows 8.1 (64 bit).

Build ID : 20170607030206
User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170607]

Comment 51

[u554753]

Verified on Windows, Mac, and Ubuntu.