Closed Bug 722196 Opened 8 years ago Closed 8 years ago

Rule view not showing media query information

Categories

(DevTools :: Inspector, defect, P1, major)

10 Branch
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 13

People

(Reporter: sys.sgx, Assigned: miker)

Details

(Whiteboard: [computedview][ruleview][fixed-in-fx-team])

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:10.0) Gecko/20100101 Firefox/10.0
Build ID: 20120123235200

Steps to reproduce:

1. Browse here: http://markdotto.com/bs2/docs/scaffolding.html
2. Inspect the element span12, it's the "12" that shows "1170px" on hover.
3. The editor does not load the correct css file, and shows 940px as the css rule applied.



Expected results:

There are two css files on this page. The responsive css most likely overwrites the rules for span class, and makes the span12 equal to 1170px. This should be shown in the style editor when inspecting the div.
Severity: normal → major
Component: Untriaged → Developer Tools: Style Editor
I can confirm this.
Status: UNCONFIRMED → NEW
Component: Developer Tools: Style Editor → Developer Tools: Inspector
Ever confirmed: true
Priority: -- → P1
QA Contact: untriaged → developer.tools.inspector
Summary: DevTools Style Editor must load the correct css file in this page → Rule view showing rules in the wrong order
Whiteboard: [ruleview]
The 1170px width rule in bootstrap-responsive.css is wrapped in a min-width media query.  It looks to me like (depending on window size) opening the sidebar is changing the size of the window, which adds in that rule.  So there are two problems that need to be fixed:

a) The rule view should give more information about rules that match a media query.
b) The style sidebar causes a resize, which mucks with the page you're inspecting.

sys.sgx, can you confirm that observation?
Yes, I can confirm this. Based on your message:

a) The rule view *must* give information about events that led to the current style being applied. (ie media queries)
b) I think this is important. Maybe chrome doesnt have this issue because the editor is not changing the size of this element. The style inspector should not split the current window, thus leading to a resize. It has to be overlayed so that the actual page is shown as intended by the css rules.
It might be good to start a new issue if you agree with the idea of having the style inspector in overlay mode.

Thanks
Assignee: nobody → mratcliffe
Status: NEW → ASSIGNED
Whiteboard: [ruleview] → [ruleview][propertyview]
Attached file Test Case
This is a simple test case that makes it possible to easily reproduce the problem:
1. Open the test case
2. Resize the browser window width to roughly the same size as the red div.
3. Right-click and inspect the div.

The side panel changes the content width, which brings in the max-width media query, which changes the div's width.
Attached patch Patch (obsolete) — Splinter Review
We still have no way to avoid reflow when the side panel is opened but that issue is something we are aware of and we have been actively seeking a solution.

The issue of not displaying any information identifying media query styles is simple to solve by slightly changing the title we display above the rule (as this patch does).
Attachment #601604 - Flags: review?(dcamp)
Whiteboard: [ruleview][propertyview] → [ruleview][propertyview][has-patch]
Whiteboard: [ruleview][propertyview][has-patch] → [ruleview][computedview][has-patch]
Attachment #601604 - Flags: review?(dcamp) → review?(jwalker)
Attachment #601604 - Flags: review?(fayearthur)
Summary: Rule view showing rules in the wrong order → Rule view not showing media query information
Comment on attachment 601604 [details] [diff] [review]
Patch

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

::: browser/devtools/styleinspector/CssRuleView.jsm
@@ +382,5 @@
>        this._title = CssLogic._strings.formatStringFromName("rule.inheritedSource",
>                                                             args, args.length);
>      }
>  
> +    return this.mediaText ? this._title + " @media " + this.mediaText : this._title;

Hey! You can shave 7 whole bytes of this. Should dramatically improve load times...
    return this._title + (this.mediaText ? " @media " + this.mediaText : "");

(Yes, I'm short of things to comment on in this patch)

::: browser/themes/gnomestripe/devtools/csshtmltree.css
@@ +141,4 @@
>    cursor: pointer;
>  }
>  
>  /* This rule is necessary because Templater.jsm breaks LTR TDs in RTL docs */

I know this isn't new, but is there something that Templater should do that it's not here?
Attachment #601604 - Flags: review?(jwalker) → review+
(In reply to Joe Walker from comment #8)
> Comment on attachment 601604 [details] [diff] [review]
> Patch
> 
> Review of attachment 601604 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/styleinspector/CssRuleView.jsm
> @@ +382,5 @@
> >        this._title = CssLogic._strings.formatStringFromName("rule.inheritedSource",
> >                                                             args, args.length);
> >      }
> >  
> > +    return this.mediaText ? this._title + " @media " + this.mediaText : this._title;
> 
> Hey! You can shave 7 whole bytes of this. Should dramatically improve load
> times...
>     return this._title + (this.mediaText ? " @media " + this.mediaText : "");
> 
> (Yes, I'm short of things to comment on in this patch)
> 

I don't care about the 7 bytes but now the line is under 80 characters so I have a warm fuzzy feeling.

> ::: browser/themes/gnomestripe/devtools/csshtmltree.css
> @@ +141,4 @@
> >    cursor: pointer;
> >  }
> >  
> >  /* This rule is necessary because Templater.jsm breaks LTR TDs in RTL docs */
> 
> I know this isn't new, but is there something that Templater should do that
> it's not here?

This is neither the time or the place but I don't care ;o)

I have never been able to work out exactly why they break. If I remember rightly, whenever anything in the xhtml template is invalid it breaks rtl for it's parent element and everything further down in the file. Adding "invalid" attributes breaks the RTLness because it invalidates the xhtml doc. Because the doc is already broken, when the template is processed it will have some rtl and some ltr. There was also something specific to td elements but I am not certain what.
Attachment #601604 - Attachment is obsolete: true
Attachment #601604 - Flags: review?(fayearthur)
OS: Windows Vista → All
Hardware: x86 → All
Whiteboard: [ruleview][computedview][has-patch] → [computedview][ruleview][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/da2e0e0fbf07
Whiteboard: [computedview][ruleview][land-in-fx-team] → [computedview][ruleview][fixed-in-fx-team]
Attachment #602045 - Attachment description: Patch 2 → [in-fx-team] Patch 2
https://hg.mozilla.org/mozilla-central/rev/da2e0e0fbf07
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.