Some strings in the XBL Bindings viewer are hard to read

RESOLVED FIXED

Status

Other Applications
DOM Inspector
--
minor
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: crussell, Assigned: crussell)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Created attachment 464669 [details]
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.
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
Assignee: nobody → Sevenspade
Status: NEW → ASSIGNED
Attachment #464671 - Flags: review?(neil)

Comment 2

7 years ago
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

7 years ago
(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.
Created attachment 465209 [details] [diff] [review]
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!
Attachment #464671 - Attachment is obsolete: true
Attachment #465209 - Flags: review?(neil)
Attachment #464671 - Flags: review?(neil)

Comment 5

7 years ago
(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

7 years ago
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.
Attachment #465209 - Flags: review?(neil) → review+
Pushed with that change:
http://hg.mozilla.org/dom-inspector/rev/eef568e4ee05
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Created attachment 466529 [details] [diff] [review]
remove unused tooltip element [Check-in comment 10]

I forgot to take this out when I switched to setting tooltipText.
Attachment #466529 - Flags: review?(neil)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 9

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

And I didn't notice either :-(
Attachment #466529 - Flags: review?(neil) → review+
Pushed:
http://hg.mozilla.org/dom-inspector/rev/05baced273f8
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
Attachment #466529 - Attachment description: remove unused tooltip element → remove unused tooltip element [Check-in comment 10]
Blocks: 582713
You need to log in before you can comment on or make changes to this bug.