933201 - [app manager] move the screenshot button in the footer

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

[Paul Rouget [:paul]]

Attachment: screenshot

Comment 1

[Gaurav Saini]

Hi, 
   
  I am willing to fix this bug.So, please I request you to assign me this bug.

Thanks,

Regards,
Gaurav Saini

Comment 2

[Paul Rouget [:paul]]

(In reply to Gaurav Saini from comment #1)
> I am willing to fix this bug.So, please I request you to assign me this bug.

Sure. Code lives there: http://mxr.mozilla.org/mozilla-central/source/browser/devtools/app-manager/

Original button is here: http://mxr.mozilla.org/mozilla-central/source/browser/devtools/app-manager/content/device.xhtml#39

We want to move it there: http://mxr.mozilla.org/mozilla-central/source/browser/devtools/app-manager/content/connection-footer.xhtml

Comment 3

[Peiyong Lin[:lpy](UTC+8)]

Hello, is Gaurav Saini still working on this?

Comment 4

[Gaurav Saini]

(In reply to Peiyong LIn from comment #3)
> Hello, is Gaurav Saini still working on this?

Yes, I am still working on this.

Comment 5

[Giovanny Gongora [:gioyik]]

Hi, 

I created a patch for this. I put the button in the footer, but i have a problem, the button didn't work. I was trying to add at the final:

<script type="application/javascript;version=1.8" src="device.js"></script>

in connection-footer.xhtml, but it didn't work. I enabled the Browser console, i can't see a warning/alert related to this. If i don't add the device.js file i got:

UI.screenshot: is not a defined function

I was trying to put the pice of code about screenshots from device.js to connection-footer.js but i didn't work. If you can give some tips and advices about what is wrong, i will really appreciate. I am going to attach the first version of the patch, if someone is interested on review it and tell me if everything is good to continue, i am going to be very happy.

Regards,
Gio

Comment 6

[Giovanny Gongora [:gioyik]]

Attachment: name.patch

Comment 7

[Paul Rouget [:paul]]

> I was trying to put the pice of code about screenshots from device.js to connection-footer.js but i didn't work.

That's the right way to do it. What didn't work?

Comment 8

[Giovanny Gongora [:gioyik]]

It didn't open a new tab and take the screenshot. Look this patch with the modifications.

Comment 9

[Giovanny Gongora [:gioyik]]

Attachment: fix.patch

Comment 10

[Paul Rouget [:paul]]

Comment on attachment 8333682 [details] [diff] [review]
fix.patch

You need to adapt the code to make it work from connection-footer.js. Here this.connected doesn't exist, getDeviceFront either. Look at the browser console, you'll see plenty of errors.

Comment 11

[Marvin Sevilla]

Hello, is Gaurav Saini working on this? If not, I would like to be assigned to this bug please.

Comment 12

[Marvin Sevilla]

Sorry, I mean is Giovanny working on this? If not, I would like to be assigned to this bug please.

Comment 13

[Meghraj Suthar (:romec)]

So, i would like to work on this bug, anyone who can assign me this one.

thank you

Regards

Meghraj Suthar

Comment 14

[Marvin Sevilla]

Attachment: rev1 - App manager moved screenshot button

I've attached my proposed patch. Let me know if this looks okay, thanks!

Comment 15

[Paul Rouget [:paul]]

Comment on attachment 8367033 [details] [diff] [review]
rev1 - App manager moved screenshot button

That's exactly what we need.
Can I just ask you to swap disconnect and screenshot? (disconnect first, then screenshot). Then we'll land your patch.

Thanks a lot!

Comment 16

[Marvin Sevilla]

Attachment: rev2 - App manager moved screenshot button

I have attached my revised patch. I swapped the disconnect and screenshot buttons (disconnect first, then screenshot). Thanks!

Comment 17

[Paul Rouget [:paul]]

Thank you Marvin!

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/204e3e268330

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/204e3e268330

Comment 20

[Will Bamberg [:wbamberg]]

App Manager is replaced by WebIDE, so I'm clearing this dev-doc-needed.