The pretty-print button within the Debugger panel has a label instead of an icon, which makes styling of it difficult. So the button should have an icon assigned via CSS like the other buttons within the devtools UI. FWIW this is needed for Firebug.[1] Sebastian [1] https://github.com/firebug/firebug.next/issues/63

Brian, this is simple to fix, but we need an icon... Any tips what icon we could use? Honza

(In reply to Jan Honza Odvarko [:Honza] from comment #1) > Brian, this is simple to fix, but we need an icon... > Any tips what icon we could use? > > Honza Stephen, I don't see any pretty print icon in the zip attached to Bug 837188. It's the '{}' button as seen here: https://hacks.mozilla.org/wp-content/uploads/2013/12/pretty-print-minified-JS.png. We are using text for the button right now, but that makes it harder to style and less consistent across operating systems. It'd be more consistent if we could use a 1x and 2x png for this, just like debugger-blackbox.png. Can you help?

> It'd be more consistent if we could use a 1x and 2x png for this General question: Why did you decide for PNGs instead of using SVG icons like Firebug does since version 2.0? Sebastian

(In reply to Sebastian Zartner from comment #3) > > It'd be more consistent if we could use a 1x and 2x png for this > General question: Why did you decide for PNGs instead of using SVG icons > like Firebug does since version 2.0? > > Sebastian I started requesting SVG for new icons when we switched to 2x in the blockers for Bug 837188, but for a lot of them it was just easier to stick with png since we already had the images in that format. I wish we were using SVG for everything because it cleans up the CSS a lot. For this case SVG would be great, but I think we would also want to do all of the other debugger buttons at the same time if we were to make the switch.

> but for a lot of them it was just easier to stick with png since we already had the images in that format. FWIW we also had our icons in PNG format earlier but we took the time to switch everything to SVG.[1] > I wish we were using SVG for everything because it cleans up the CSS a lot. For this case SVG would > be great, but I think we would also want to do all of the other debugger buttons at the same time if > we were to make the switch. So should I file a bug for switching all icons to SVG? Sebastian [1] https://code.google.com/p/fbug/issues/detail?id=7000

(In reply to Sebastian Zartner from comment #5) > So should I file a bug for switching all icons to SVG? Sure, probably the most up to date collection of images that need to be converted is from the repository (https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/devtools/images/), rather than one of the zips from a previous bug, as we've added and modified quite a few images as one-offs in the meantime.

Normal resolution prettyprint icon

High resolution prettyprint icon

Replace the label by the icon

This is my very first patch against Mozilla code, so Brian, could you please review and check if I did it right and guide me with the next steps to do? Note that I had to add my name to the patch manually. Not sure how to get Mercurial to do that for me. Sebastian

(In reply to Sebastian Zartner from comment #10) > This is my very first patch against Mozilla code, so Brian, could you please > review and check if I did it right and guide me with the next steps to do? Sure! The patch applies fine, but please add the new image files to the patch with `hg add`. Also, when you reupload the patch, add r=bgrins at the end of the commit message and upload with me in the r? field. > Note that I had to add my name to the patch manually. Not sure how to get > Mercurial to do that for me. See this document https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F, it will help you get that set up.

>> Note that I had to add my name to the patch manually. Not sure how to get >> Mercurial to do that for me. > > See this document https://developer.mozilla.org/en-US > /docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F, it will help > you get that set up. I read that before I uploaded my first patch, though it didn't work. I guess my problem could be related to bug 1077326 as I got the same errors. Anyway, I hope it works for you now. Sebastian

Comment on attachment 8515125 [details] [diff] [review] prettyprint2.patch Review of attachment 8515125 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=2eac4b0444ff

If I interpret those test results correctly, the failing tests are unrelated. (It's a minor layout change, anyway.) So what's next? Somehow I need to request a check-in, right? Sebastian

(In reply to Sebastian Zartner from comment #14) > If I interpret those test results correctly, the failing tests are > unrelated. (It's a minor layout change, anyway.) > So what's next? Somehow I need to request a check-in, right? > > Sebastian Yeah, you can add the checkin-needed keyword to the bug