The default bug view has changed. See this FAQ.

Style Inspector's left column should auto-fit text contents

RESOLVED FIXED in Firefox 12

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: miker, Unassigned)

Tracking

unspecified
Firefox 12
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [styleinspector])

Attachments

(1 attachment, 3 obsolete attachments)

The style inspector's left column should auto-fit it's contents. The contents of the right-column should wrap as on small screens values are not always visible.

Mihai created a patch for this in another bug but because of bug 460637 we could not use it. If the patch now works we will use it ... otherwise it would make a great test case for bug 460637.
Created attachment 575860 [details] [diff] [review]
Flexible Column Widths (bitrotted)
Created attachment 576928 [details] [diff] [review]
Flexible Column Widths

Wow, that took longer than expected partly because of changes that we had made to the code, partly due to xul quirks and partly because I am sick and my brain is in standby mode this week.

I have updated the patch and removed a lot of cruft from the css files. I have also logged bug 705276 to separate content & document css.

This bug does raise the "invalid BC damage area" assertion which in turn logs hundreds of stack traces. This slows the test seriously enough to make it time out and fail so I will explain how to reproduce it in bug 460637 so that it can be fixed.

Joe, can you look over the css and make sure I am not unnecessarily breaking any Daoist rules?
Attachment #575860 - Attachment is obsolete: true
Attachment #576928 - Flags: review?(jwalker)
Depends on: 460637
Comment on attachment 576928 [details] [diff] [review]
Flexible Column Widths

Review of attachment 576928 [details] [diff] [review]:
-----------------------------------------------------------------

I can only give feedback right now, so f+ nor r+. I'm assuming that the padding:0's are either needed or will be removed.
Dao will need to take a look.

::: browser/devtools/styleinspector/CssHtmlTree.jsm
@@ +634,4 @@
>    {
>      let doc = this.tree.doc;
> +    this.element = doc.createElementNS(HTML_NS, "tr");
> +    this.element.setAttribute("class", this.propertyHeaderClassName);

Minor, not something I would demand you go an change everywhere, but classList rocks imho.

::: browser/themes/gnomestripe/devtools/csshtmltree.css
@@ +49,5 @@
>  }
>  
>  
>  .property-header {
> +  padding: 0;

Do we actually need this? Isn't it the default?
Attachment #576928 - Flags: feedback+
Attachment #576928 - Flags: review?(jwalker)

Comment 4

5 years ago
Michael, if you see still the assert once bug 460637 is fixed please file a bug and CC me.
(In reply to Joe Walker from comment #3)
> Comment on attachment 576928 [details] [diff] [review] [diff] [details] [review]
> Flexible Column Widths
> 
> Review of attachment 576928 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> I can only give feedback right now, so f+ nor r+. I'm assuming that the
> padding:0's are either needed or will be removed.
> Dao will need to take a look.
> 
> ::: browser/devtools/styleinspector/CssHtmlTree.jsm
> @@ +634,4 @@
> >    {
> >      let doc = this.tree.doc;
> > +    this.element = doc.createElementNS(HTML_NS, "tr");
> > +    this.element.setAttribute("class", this.propertyHeaderClassName);
>  
> Minor, not something I would demand you go an change everywhere, but
> classList rocks imho.
> 

I agree but this is how it has been done in this section of the code so I chose not to change it.
> ::: browser/themes/gnomestripe/devtools/csshtmltree.css
> @@ +49,5 @@
> >  }
> >  
> >  
> >  .property-header {
> > +  padding: 0;
> 
> Do we actually need this? Isn't it the default?

It is not the default and we do need it.

Thanks for the feedback.
Comment on attachment 576928 [details] [diff] [review]
Flexible Column Widths

Dão, can you review this as it is all style related?
Attachment #576928 - Flags: review?(dao)
Depends on: 702861
Created attachment 577590 [details] [diff] [review]
Flexible Column Widths

Rebased due to failing windows test
Attachment #576928 - Attachment is obsolete: true
Attachment #576928 - Flags: review?(dao)
Attachment #577590 - Flags: review?(dao)
Whiteboard: [styleinspector] → [styleinspector][has-patch]
Comment on attachment 577590 [details] [diff] [review]
Flexible Column Widths

This seems to regress the expander styling, i.e. they expanders are cut off again... Tested on Ubuntu 11.10.
Attachment #577590 - Flags: review?(dao) → review-
Duplicate of this bug: 697398
Blocks: 589375
Created attachment 582800 [details] [diff] [review]
Flexible Column Widths

(In reply to Dão Gottwald [:dao] from comment #8)
> Comment on attachment 577590 [details] [diff] [review]
> Flexible Column Widths
> 
> This seems to regress the expander styling, i.e. they expanders are cut off
> again... Tested on Ubuntu 11.10.

Fixed
Attachment #577590 - Attachment is obsolete: true
Attachment #582800 - Flags: review?(dao)

Updated

5 years ago
Attachment #582800 - Flags: review?(dao) → review+
Whiteboard: [styleinspector][has-patch] → [styleinspector][land-in-fx-team]

Updated

5 years ago
Attachment #582800 - Flags: review+

Comment 11

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/35d39a331bb8
Whiteboard: [styleinspector][land-in-fx-team] → [styleinspector][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/35d39a331bb8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [styleinspector][fixed-in-fx-team] → [styleinspector]
Target Milestone: --- → Firefox 12
You need to log in before you can comment on or make changes to this bug.