tighten up the "Launch Application" dialog (uses too much vertical space) 1) click on ttp://google.com 2) get the new "Launch Application" dialog I think this dialog needs some cleanup. a) "ttp" line is too tall b) "Choose an Application" line is too tall c) "Choose..." button is too tall screen shot coming.

I'm using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007072605 Minefield/3.0a7pre

ttp is supposed to have an image next to it - but we can't do that just yet (that's why it's as tall as it is)

Maybe. But the button doesn't have to be as tall as the whole line, does it? PS: When I select "ttp" and click OK nothing happens. I really don't understand this dialog. What is supposed to happen then? There is no ttp-handler on my System...

> When I select "ttp" and click OK nothing happens. see bug #389969

Do you also want to use an em-based width (rather than px-based) so the dialog can handle changing system font sizes? http://mxr.mozilla.org/mozilla/source/toolkit/mozapps/handling/content/dialog.xul#48

Does the width attribute take em sizes? If not, probably not since we want localizers to be able to change the width...

Yes, it will take em sizes. For example, it looks like the "Change Action" dialog is doing it that way. They're putting the actual em value in a .dtd so it can be localized. http://mxr.mozilla.org/mozilla/source/browser/components/preferences/changeaction.xul#54 http://mxr.mozilla.org/mozilla/source/browser/locales/en-US/chrome/browser/preferences/changeaction.dtd#2

I'm not so sure I like the idea now that I think about it more. What happens if the font increase makes the dialog too wide?

Several dialogs define width in terms of an em size (e.g. Options window). Maybe someone from accessibility could advise. What would someone who is using large fonts prefer to have happen?

ui feedback wanted as well (beltzner/mconnor)

too wide is better than too narrow, we should use em everywhere (we do in most places) Not sure if we want to tighten up the vertical space though.

Not sure if was due to the latest efforts of minimizing the Launch Application window, but it got too short. I've attached a screen shot which shows this symptom. I'm using "Classic" theme (Windows 2000 style) in Windows XP with but I'm sure this isn't the cause for that - default "Blue" theme takes much more space for almost every widget (title bars, buttons, etc.)

Taking, as I'm trying to find owners for the various unowned Firefox 3 blocks. However! I've heard this crazy rumor that mfinkle might have a patch in the works for this bug. If that's the case (or if you'd like it to be, Mark), feel free to grab this bug from me!

I'll take this one - you work on the hard ones :)

moving out bugs that don't need to block b1

This patch: * reduces the height of the richlistbox "button" chooser row * adds support for fetching the icon for default handlers * changes dialog width and height to "em" * removes a period "." from the checkbox label * removes the unused <description> from the richlistbox helper XBL

This doesn't quite look right still on the mac (although I'm not sure if you intended to fix Bug 396635 in the process of this or not). Note the scrollbars showing up on the mac.

Comment on attachment 283597 [details] [diff] [review] fixes vertical size of row and adds support for handler icon future note - please diff from toplevel (in mozilla/, not in mozilla/toolkit/) >+ >+ // Try to get some icon for this handler >+ let mimeSvc = Cc["@mozilla.org/uriloader/external-helper-app-service;1"]. >+ getService(Ci.nsIMIMEService); >+ let mimeType = "text/html"; //XXX dumb init for now >+ try { >+ mimeType = mimeSvc.getTypeFromURI(this._URI); >+ } >+ catch (ex) {} >+ >+ try { >+ let mimeInfo = mimeSvc.getFromTypeAndExtension(mimeType, null); >+ if (mimeInfo instanceof Ci.nsIPropertyBag) { >+ let url = mimeInfo.getProperty("defaultApplicationIconURL"); >+ if (url) >+ elm.setAttribute("image", url + "?size=32"); >+ } >+ } >+ catch(ex) {} Something seems wrong about all of this. If we don't know what the type is, we'll use text/html. Wouldn't it be better to set it to null, and not do the second try/catch if it's null? >-richlistitem { >+richlistitem[type] { > min-height: 36px; /* Don't forget to update the richlistbox height! */ > } Why the change? I guess I don't follow it.. > >+richlistitem { >+ -moz-box-align: center; >+} Same here With these addressed, I'd suggest Mano or Gavin for reviewers (my review doesn't count for toolkit unless it's a DM patch).

(In reply to comment #19) > (From update of attachment 283597 [details] [diff] [review]) > >+ > >+ // Try to get some icon for this handler > >+ let mimeSvc = Cc["@mozilla.org/uriloader/external-helper-app-service;1"]. > >+ getService(Ci.nsIMIMEService); > >+ let mimeType = "text/html"; //XXX dumb init for now > >+ try { > >+ mimeType = mimeSvc.getTypeFromURI(this._URI); > >+ } > >+ catch (ex) {} > >+ > >+ try { > >+ let mimeInfo = mimeSvc.getFromTypeAndExtension(mimeType, null); > >+ if (mimeInfo instanceof Ci.nsIPropertyBag) { > >+ let url = mimeInfo.getProperty("defaultApplicationIconURL"); > >+ if (url) > >+ elm.setAttribute("image", url + "?size=32"); > >+ } > >+ } > >+ catch(ex) {} > Something seems wrong about all of this. If we don't know what the type is, > we'll use text/html. Wouldn't it be better to set it to null, and not do the > second try/catch if it's null? > We need to pick an icon if we can't determine the type. I am happy to use any, even the default application.png, just not blank. > >-richlistitem { > >+richlistitem[type] { > > min-height: 36px; /* Don't forget to update the richlistbox height! */ > > } > Why the change? I guess I don't follow it.. We don't want the "Choose and application" row to be 36px tall, only the rows that are "type" of application. > > > >+richlistitem { > >+ -moz-box-align: center; > >+} > Same here > We want the text to be centered vetically in the row. The image and/or the button is taller than the text so the text appears aligned to the top of the row. The UI mockups had the text centered vertically in the row. > With these addressed, I'd suggest Mano or Gavin for reviewers (my review > doesn't count for toolkit unless it's a DM patch). > I will use a better default application image, then submit for review again.

If we don't already have a handler or protocol specific image (I'll spin off a separate bug about that), I'm not convinced that showing some generic image is terribly helpful to the user, so I wouldn't sweat that.

Same as previous patch except: * dropped any changes for dialog resizing (those were fixed in another bug) * added CSS changes for Mac

bug 404546 added so we can get a decent icon for the default handler

(In reply to comment #17) > * removes the unused <description> from the richlistbox helper XBL It will probably be used to display hostnames of web handlers (see bug 402620)

This patch changes the CSS to reduce the height of the "button" row of the richlistitems. It also removes the currently unused description label in the richlistitem row and vertically center aligns the remaining name label, since the row is 32px high (from the image)

Checking in toolkit/mozapps/handling/content/handler.xml; /cvsroot/mozilla/toolkit/mozapps/handling/content/handler.xml,v <-- handler.xml new revision: 1.2; previous revision: 1.1 done Checking in toolkit/themes/pinstripe/mozapps/handling/handling.css; /cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/handling/handling.css,v <-- handling.css new revision: 1.2; previous revision: 1.1 done Checking in toolkit/themes/winstripe/mozapps/handling/handling.css; /cvsroot/mozilla/toolkit/themes/winstripe/mozapps/handling/handling.css,v <-- handling.css new revision: 1.2; previous revision: 1.1 done

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2) Gecko/2007121014 Firefox/3.0b2. Cancel and OK buttons now fit completely in the window, and the scrollbar is gone.