722196 - Rule view not showing media query information

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[sys.sgx]

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.

Comment 1

[Dave Camp (:dcamp)]

I can confirm this.

Comment 2

[Dave Camp (:dcamp)]

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?

Comment 3

[sys.sgx]

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.

Comment 4

[sys.sgx]

New one found: https://bugzilla.mozilla.org/show_bug.cgi?id=722446

Comment 5

[sys.sgx]

It might be good to start a new issue if you agree with the idea of having the style inspector in overlay mode.

Thanks

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: 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.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Patch

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).

Comment 8

[Joe Walker [:jwalker]]

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?

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: [in-fx-team] Patch 2

(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.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/da2e0e0fbf07

Comment 11

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/da2e0e0fbf07