1534180 - [remote-dbg-next] Change the icon representing processes

Status: VERIFIED FIXED

(DevTools :: about:debugging, enhancement, P1)

Description

[Daisuke Akatsuka (:daisuke)]

This is a followup bug of bug 1522062.
In this bug, we use settings.svg for processes. However, since this icon will use to indicate Setup page, we might be better to change.

Comment 1

[Julian Descottes [:jdescottes]]

H Victoria, for the moment we re-used the "Settings" cog icon for the "Processes" debug target. Any idea how we could get a better icon?

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: image.png

Comment 3

[Ola Gasidlo [:ogasidlo] [:ola] OOO - please ping :ladybenko:]

To fix this bug, following things need to be done:

1. Duplicate following icon:
   https://searchfox.org/mozilla-central/source/devtools/client/themes/images/command-console.svg
   and name it aboutdebugging-command-console.svg

2. Add the path of the icon to https://searchfox.org/mozilla-central/source/devtools/client/jar.mn#77
   Please make sure to watch out for the alphabetic order.

[Re-run the build with ./mach build. Otherwise you will not see the icon when you replace it.]

3. Replace settings.svg with aboutdebugging-command-console.svg in line:
   https://searchfox.org/mozilla-central/source/devtools/client/aboutdebugging-new/src/components/sidebar/Sidebar.js#24

Note:
The decision on the icon is not final yet. Please wait for feedback from Victoria to grab this bug.

Comment 4

[Victoria Wang [:victoria]]

Hi all, the console icon sounds good to me as a replacement. Thanks!

Comment 5

[danielleleb12]

Hi, I'd like to work on this! Is there a specific mercurial command I should be use for duplicating the icon file, or should I just do it manually?

Comment 6

[Ola Gasidlo [:ogasidlo] [:ola] OOO - please ping :ladybenko:]

Hi Danielle!

You could navigate to the folder with the cd command (docs for cd command) and then copy the file with cp command-console.svg aboutdebugging-command-console.svg (docs for cp command) or copy + rename it manually.

Comment 7

[Julian Descottes [:jdescottes]]

(In reply to danielleleb12 from comment #5)

Hi, I'd like to work on this! Is there a specific mercurial command I should be use for duplicating the icon file, or should I just do it manually?

Assigning to you! Thanks for offering to work on this :)

The Processes debug category is hidden by default and only displayed when connected to a remote browser.
If you want to actually see the icon in the UI, you need to:

• go to about:config, set devtools.aboutdebugging.process-debugging to true
• connect to a remote runtime in about:debugging-new (documentation, screencast).

If you have trouble with connecting to a remote runtime, you can also just comment out the following line: debug-target-support.js#39. It will allow the "Processes" category to be displayed when clicking on "This Nightly". The feature doesn't really work there, but that's good enought to test the icon

We will also have to add a line in https://searchfox.org/mozilla-central/source/browser/installer/allowed-dupes.mn because we are duplicating an existing icon, but we can give pointers about that in the review.

Comment 8

[danielleleb12]

Thank you both for all of the information!

I completed the steps as described, but wasn't able to see the changed icon, using this method to test it:

If you have trouble with connecting to a remote runtime, you can also just comment out the following line: debug-target-support.js#39. It will allow the "Processes" category to be displayed when clicking on "This Nightly". The feature doesn't really work there, but that's good enought to test the icon

However, with some investigating I saw that it seems that the icon in this case comes from RuntimePage.js, line 74 (after making the change here, I can now see the new icon).

Should I change the icon from settings.svg to aboutdebugging-command-console.svg there as well?

Comment 9

[Julian Descottes [:jdescottes]]

Thanks Danielle! Sorry I missed your question.

However, with some investigating I saw that it seems that the icon in this
case comes from RuntimePage.js, line 74 (after making the change here, I can now see the new icon).
Should I change the icon from settings.svg to aboutdebugging-command-console.svg there as well?

Actually, it should only be changed in RuntimePage.js :) The one defined in Sidebar.js should remain settings.svg, sorry about this!

(If you want to get feedback faster, don't hesitate to ping us on slack or use the "need more information" checkbox right below the comment section here on Bugzilla)

Comment 10

[danielleleb12]

Attachment: Bug 1534180 - Changes processes icon.

Comment 11

[Tim Nguyen :ntim]

(In reply to Julian Descottes [:jdescottes] from comment #7)

We will also have to add a line in https://searchfox.org/mozilla-central/source/browser/installer/allowed-dupes.mn because we are duplicating an existing icon, but we can give pointers about that in the review.

The top of that file makes it pretty clear that we shouldn't add new items to the list, to avoid unnecessarily increasing the installer size.

Comment 12

[Julian Descottes [:jdescottes]]

(In reply to Tim Nguyen :ntim from comment #11)

(In reply to Julian Descottes [:jdescottes] from comment #7)

We will also have to add a line in https://searchfox.org/mozilla-central/source/browser/installer/allowed-dupes.mn because we are duplicating an existing icon, but we can give pointers about that in the review.

The top of that file makes it pretty clear that we shouldn't add new items to the list, to avoid unnecessarily increasing the installer size.

Yes, but in this case reusing the icon means we are linking two icons that have a different purpose:
1/ represent a command line
2/ represent a process

So reusing the icon here means that next time the console icon is updated, the process icon would be updated as well which we don't necessarily want to do. We must balance maintenance cost and installer size, do you have a constructive proposition to address both?

Comment 13

[Tim Nguyen :ntim]

(In reply to Julian Descottes [:jdescottes] from comment #12)

(In reply to Tim Nguyen :ntim from comment #11)

(In reply to Julian Descottes [:jdescottes] from comment #7)

We will also have to add a line in https://searchfox.org/mozilla-central/source/browser/installer/allowed-dupes.mn because we are duplicating an existing icon, but we can give pointers about that in the review.

The top of that file makes it pretty clear that we shouldn't add new items to the list, to avoid unnecessarily increasing the installer size.

Yes, but in this case reusing the icon means we are linking two icons that have a different purpose:
1/ represent a command line
2/ represent a process

So reusing the icon here means that next time the console icon is updated, the process icon would be updated as well which we don't necessarily want to do.

Fair enough, but in this case, we should name the new icon process.svg rather than aboutdebugging-command-console.svg. The current name gives the impression that aboutdebugging-command-console.svg and command-console.svg have the same purpose...

We must balance maintenance cost and installer size, do you have a constructive proposition to address both?

We could probably pre-process the files and use #include to avoid duplication, but that's unnecessarily complicated for this purpose.

Anyway, I'd rather not add a new entry to allowed-dupes.mn, simply because it doesn't cause a failure when an entry becomes unused, see bug 1518445. The result is that the file becomes filled with unused entries when they are de-duplicated (which is already case right now).

How about just shuffling the SVG attributes to make both icons different ?

Comment 14

[Julian Descottes [:jdescottes]]

How about just shuffling the SVG attributes to make both icons different ?

Works for me :)

Fair enough, but in this case, we should name the new icon process.svg rather than aboutdebugging-command-console.svg. The current name gives the impression that aboutdebugging-command-console.svg and command-console.svg have the same purpose...

Yes I agree, mentioned it on the review now, and will take care of it in a followup if needed.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1a6d6ea370e
Changes processes icon. r=jdescottes

Comment 16

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/c1a6d6ea370e

Comment 17

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox Nightly 69.0a1 (2019-06-17) and on Firefox 68.0b11 on Windows 10 x 64, Mac OS X 10.14 and on Ubuntu 18.04 x64.