Bug 1007061 (rm-am)

Remove /browser/devtools/app-manager/

RESOLVED FIXED in Firefox 44

Status

RESOLVED FIXED
5 years ago
10 months ago

People

(Reporter: paul, Assigned: ochameau)

Tracking

Trunk
Firefox 44
x86
All
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

5 years ago
One the new app manager has landed, we want to remove the old code. We need to make sure the new code doesn't depend on the old code anymore.
Filter on 86b7095e-2bd0-499e-a704-d00f2524aeef / PAUL STOP SETTING QA CONTACT TO THE DEVTOOLS COMPONENT'S WATCHERS EMAIL FOR BUGS YOU FILE :)
QA Contact: developer.tools
(Reporter)

Updated

5 years ago
Duplicate of this bug: 980919
(Reporter)

Updated

5 years ago
Duplicate of this bug: 980335
Duplicate of this bug: 1062561
Duplicate of this bug: 985287
(Assignee)

Comment 6

5 years ago
And we need to ensure WebIDE is as much usage as app-manager from gaia developers perspective.
I think most issues have been highlighted in bug 1055279 comments.
Depends on: 1070957
A deprecation notice was added to App Manager in 35 that points you to WebIDE (bug 1070957).

I think it would be good to resolve bug 1055347 (searching for apps) before removing App Manager, as that was a vocal complaint from Gaia devs.

Other than that, we've migrated everything to WebIDE.

Can anyone think of other tasks to complete before removing App Manager?

I will be sure to announce it's impending removal before actually removing it.
Flags: needinfo?(poirot.alex)
Flags: needinfo?(paul)
Flags: needinfo?(janx)
(Reporter)

Comment 8

4 years ago
Reducing the number of required clicks is important too. I think this bug should depend on bug 1079347.
Flags: needinfo?(paul)
(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #8)
> Reducing the number of required clicks is important too. I think this bug
> should depend on bug 1079347.

Right, I assumed "searching for apps" would also be achieved via that bug, but yes, let's depend on it as well.
Depends on: 1079347
(Assignee)

Comment 10

4 years ago
I see two things, one "simple" the app selection you just mentioned and one another, harder, really specific to gaia devs.
That's about auto-connect to last runtime.
When you flash gaia with `make install-gaia`, or `APP=system make install-gaia`, it reboots b2g without reseting adb. The auto-connect feature doesn't work as the runtime stays on but we just loose the debugger connection.
With that behavior, in some cases, when working on gaia, you have to click more (2 additional clicks, involving a popup) each time you update your device. In the app manager you just had to hit connect once, without any popup.
I'm not sure it is mandatory to fix that, but that would be really nice to have.
Flags: needinfo?(poirot.alex)
(In reply to Alexandre Poirot [:ochameau] from comment #10)
> I see two things, one "simple" the app selection you just mentioned and one
> another, harder, really specific to gaia devs.
> That's about auto-connect to last runtime.
> When you flash gaia with `make install-gaia`, or `APP=system make
> install-gaia`, it reboots b2g without reseting adb. The auto-connect feature
> doesn't work as the runtime stays on but we just loose the debugger
> connection.
> With that behavior, in some cases, when working on gaia, you have to click
> more (2 additional clicks, involving a popup) each time you update your
> device. In the app manager you just had to hit connect once, without any
> popup.
> I'm not sure it is mandatory to fix that, but that would be really nice to
> have.

Okay, I will tentatively depend on that as well, since it's another version of the "too many clicks" problem.
Depends on: 1055278
App search would be a big plus, but else removing App Manager seems fine.

(I guess the files app-projects.js, app-validator.js, plugin.js, default-app-icon.png, rocket.svg and noise.png will be moved into the respective webide/ folders?)

One thing an app developer told me is that their team still use the old "Firefox OS 1.1 Simulator" addon, but they were happy about WebIDE in general. Also, they were having trouble debugging marketplace apps on desktop (I'll see what the problem is and file bugs if necessary).
Flags: needinfo?(janx)
Alias: rm-am
Depends on: 1100484
(Assignee)

Comment 14

4 years ago
Posted patch Remove app-manager. (obsolete) — Splinter Review
Here is a first step, remove everything but what is still used by WebIDE.
(Assignee)

Comment 16

4 years ago
Posted patch patch v2 (obsolete) — Splinter Review
Cleanup even more stuff from browser/.
Attachment #8671363 - Attachment is obsolete: true
(Assignee)

Comment 17

4 years ago
Posted patch patch v3 (obsolete) — Splinter Review
Rebased, already a (simple) conflict :x

But I still see an issue with WebIDE project editor being empty dependeing on file type.
Unfortunately I got an exception (with no stack!!! Trying to figure out why/how)
that just says "Error" (so the stack is even more needed!).
Attachment #8671393 - Attachment is obsolete: true
(Assignee)

Comment 20

4 years ago
Posted patch patch v4 (obsolete) — Splinter Review
Rebased. Looks like the exception vanished... I may be based on a broken gecko changeset?

Looks like try is happy.
Do you think that's the right time to proceed?
Should we wait for about:debugging to support devices/simulators?

We can then followup by moving modules still used by webide
or stop using them.
Attachment #8671619 - Attachment is obsolete: true
Attachment #8673116 - Flags: feedback?(jryans)
Comment on attachment 8673116 [details] [diff] [review]
patch v4

Review of attachment 8673116 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/app-manager/moz.build
@@ +7,4 @@
>  MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini']
>  
>  DevToolsModules(
>      'app-projects.js',

Do you intend to clean up things like this by moving to WebIDE in a separate step?

::: devtools/client/jar.mn
@@ -345,5 @@
> -    skin/themes/app-manager/images/plus.svg (themes/app-manager/images/plus.svg)
> -    skin/themes/app-manager/images/remove.svg (themes/app-manager/images/remove.svg)
> -    skin/themes/app-manager/images/add.svg (themes/app-manager/images/add.svg)
> -    skin/themes/app-manager/images/index-icons.svg (themes/app-manager/images/index-icons.svg)
> -    skin/themes/app-manager/images/rocket.svg (themes/app-manager/images/rocket.svg)

WebIDE uses rocket and noise[1]

[1]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/themes/webide.css#137

@@ -347,5 @@
> -    skin/themes/app-manager/images/add.svg (themes/app-manager/images/add.svg)
> -    skin/themes/app-manager/images/index-icons.svg (themes/app-manager/images/index-icons.svg)
> -    skin/themes/app-manager/images/rocket.svg (themes/app-manager/images/rocket.svg)
> -    skin/themes/app-manager/images/noise.png (themes/app-manager/images/noise.png)
> -    skin/themes/app-manager/images/default-app-icon.png (themes/app-manager/images/default-app-icon.png)

WebIDE uses default-app-icon[1]

[1]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/modules/app-manager.js#33
Attachment #8673116 - Flags: feedback?(jryans) → feedback+
(Assignee)

Comment 22

4 years ago
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #21)
> Comment on attachment 8673116 [details] [diff] [review]
> patch v4
> 
> Review of attachment 8673116 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/client/app-manager/moz.build
> @@ +7,4 @@
> >  MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini']
> >  
> >  DevToolsModules(
> >      'app-projects.js',
> 
> Do you intend to clean up things like this by moving to WebIDE in a separate
> step?
> 

Yes, let's first prune the unnecessary files. Then see resource by resource if each is required and how to refactor or just move files around.
(Assignee)

Comment 23

4 years ago
Just sent a note on devtools and fxos mailing list:
  https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/yRJbSzvsODg
I don't see it on dev-fxos yet, but I hope it will appear there also!
Assignee: nobody → poirot.alex
(Assignee)

Comment 24

4 years ago
Posted patch patch v5 (obsolete) — Splinter Review
Attachment #8673116 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8675601 - Flags: review?(jryans)
(Assignee)

Comment 26

4 years ago
Just some more latency on fxos, here is the note:
  https://groups.google.com/forum/#!topic/mozilla.dev.fxos/883sAxUjDoc
Comment on attachment 8675601 [details] [diff] [review]
patch v5

Review of attachment 8675601 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me! :D

If you don't plan to do it in this bug, please file a follow up to clean up the remaining files still used by WebIDE.
Attachment #8675601 - Flags: review?(jryans) → review+
(Assignee)

Comment 28

4 years ago
Yes, I plan to do it in followups. I would like to just wait for you resource://devtools effort to land before landing this.
(Assignee)

Updated

4 years ago
Blocks: 1216590
(Assignee)

Comment 29

4 years ago
Posted patch patch v6Splinter Review
Rebased after the resource://devtools file move.
Attachment #8675601 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #8677371 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c0732e791208
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.