586149 - Some strings in the XBL Bindings viewer are hard to read

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: Binding URL strings getting cropped in a way that makes it hard to determine binding files and ids. Variable width font in the source view.

Menulist and menuitem labels are often too long. It wouldn't be so bad, but the interesting bits are what gets cropped.

The rules for monospace text in the source views disappeared from the final patch from bug 530643.

Comment 1

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: Center cropping and tooltips for menulist and menuitems. Monospaced font on source view.

There's also an unrelated change to deal with this warning:

Warning: assignment to undeclared variable kids
Source File: chrome://controlinspector/content/viewers/xblBindings/xblBindings.js
Line: 347

Comment 2

[[e-mail]]

Comment on attachment 464671 [details] [diff] [review]
Center cropping and tooltips for menulist and menuitems. Monospaced font on source view.

>+    <tooltip id="ttBindings"/>
Assuming it works, I'd prefer
<tooltip id="ttBindings">
  <observes element="mlBindings" attribute="label"/>
</tooltip>

Or add some code to displayBinding to update the menulist tooltiptext?

Comment 3

[[e-mail]]

(In reply to comment #1)
> Monospaced font on source view.
I don't actually like this. Maybe I'm used to Arial, but I also notice that the fixed font also takes up much more space and therefore you can see less code at one time and the code you do see is more likely to get line-wrapped.

Comment 4

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: Center cropping and tooltips for menulist and menuitems; no monospace for source view.

(In reply to comment #2)
> >+    <tooltip id="ttBindings"/>
> Assuming it works, I'd prefer
> <tooltip id="ttBindings">
>   <observes element="mlBindings" attribute="label"/>
> </tooltip>

I prefer the declarative approach, but the observes being a child of the tooltip messes up the tooltip's binding or something. It doesn't get its anonymous label child then.

> Or add some code to displayBinding to update the menulist tooltiptext?

This is what I did first, but it didn't work. It is now. Here we go then.

(In reply to comment #1)
> Created attachment 464671 [details] [diff] [review]
> Center cropping and tooltips for menulist and menuitems. Monospaced font on
> source view.
> 
> There's also an unrelated change to deal with this warning:
> 
> Warning: assignment to undeclared variable kids
> Source File:
> chrome://controlinspector/content/viewers/xblBindings/xblBindings.js
> Line: 347

Hey, look, this patch even includes this change this time!

Comment 5

[[e-mail]]

(In reply to comment #4)
> (In reply to comment #2)
> > >+    <tooltip id="ttBindings"/>
> > Assuming it works, I'd prefer
> > <tooltip id="ttBindings">
> >   <observes element="mlBindings" attribute="label"/>
> > </tooltip>
> I prefer the declarative approach, but the observes being a child of the
> tooltip messes up the tooltip's binding or something. It doesn't get its
> anonymous label child then.
How unfortunate. I notice that <button> and <toolbarbutton> has special XBL to stop that from happening. Maybe <tooltip observes="mlBindings"> would work?

Comment 6

[[e-mail]]

Comment on attachment 465209 [details] [diff] [review]
Center cropping and tooltips for menulist and menuitems; no monospace for source view.

>       var url = urls.queryElementAt(i, Components.interfaces.nsIURI).spec;
>-      menulist.appendItem(url, url);
>+      var currentItem = this.mBindingsList.appendItem(url, url);
>+      currentItem.crop = "center";
>+      currentItem.tooltipText = currentItem.value;
Seems to make more sense to use = url again here.

Comment 7

[Colby Russell :crussell (no longer Mozilla-ing)]

Pushed with that change:
http://hg.mozilla.org/dom-inspector/rev/eef568e4ee05

Comment 8

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: remove unused tooltip element [Check-in comment 10]

I forgot to take this out when I switched to setting tooltipText.

Comment 9

[[e-mail]]

Comment on attachment 466529 [details] [diff] [review]
remove unused tooltip element [Check-in comment 10]

And I didn't notice either :-(

Comment 10

[Colby Russell :crussell (no longer Mozilla-ing)]

Pushed:
http://hg.mozilla.org/dom-inspector/rev/05baced273f8