Last Comment Bug 747225 - [layout view] Show when the margin have the "auto" value
: [layout view] Show when the margin have the "auto" value
Status: RESOLVED FIXED
[fixed-in-fx-team]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Firefox 15
Assigned To: Dave Camp (:dcamp)
:
:
Mentors:
Depends on:
Blocks: 747218
  Show dependency treegraph
 
Reported: 2012-04-19 15:53 PDT by Paul Rouget [:paul]
Modified: 2012-05-26 10:24 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v1 (10.91 KB, patch)
2012-05-10 05:31 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
patch v2 (8.27 KB, patch)
2012-05-24 11:16 PDT, Dave Camp (:dcamp)
paul: review+
Details | Diff | Splinter Review

Description Paul Rouget [:paul] 2012-04-19 15:53:34 PDT

    
Comment 1 Paul Rouget [:paul] 2012-05-09 09:27:51 PDT
So what is the right approach here?

I use the computed style for the border/padding/margins.

To figure out when the margin is auto, I need to start using the node rules and find which
one applies. Should I do that for all the values?
Comment 2 Paul Rouget [:paul] 2012-05-09 09:56:20 PDT
I think I just need to get the "bestmatch" value from the property view.
Comment 3 Paul Rouget [:paul] 2012-05-10 05:31:17 PDT
Created attachment 622691 [details] [diff] [review]
patch v1
Comment 4 Paul Rouget [:paul] 2012-05-14 02:55:39 PDT
I am not confident about this approach. There might be a shorter path.
Comment 5 Dave Camp (:dcamp) 2012-05-14 13:53:51 PDT
Comment on attachment 622691 [details] [diff] [review]
patch v1

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

If parsing the css styles is the best way to find this, maybe you could reuse ElementStyle from CssRuleView?

You can create an ElementStyle with your node, it'll do all the parsing you need.

ElementStyle.rules holds a list of Rule objects,
Rule.textProps has a list of TextProperty
TextProperty.computed has a list of the expanded shorthand properties.

Would that help you at all?
Comment 6 Michael Ratcliffe [:miker] [:mratcliffe] 2012-05-21 06:31:57 PDT
Comment on attachment 622691 [details] [diff] [review]
patch v1

Looks good but Dave does have a point ... using ElementStyle would simplify things slightly.
Comment 7 Dave Camp (:dcamp) 2012-05-24 11:16:20 PDT
Created attachment 626891 [details] [diff] [review]
patch v2

ElementStyle would have simplified things a bit, but CssLogic simplifies them a lot.
Comment 8 Paul Rouget [:paul] 2012-05-24 12:42:08 PDT
Comment on attachment 626891 [details] [diff] [review]
patch v2

Much better :) Thanks.

># HG changeset patch
># User Paul Rouget <paul@mozilla.com>

You might want to change that.

>+const Cc = Components.classes;
>+const Ci = Components.interfaces;
> const Cu = Components.utils;
> Cu.import("resource://gre/modules/Services.jsm");
> Cu.import("resource:///modules/inspector.jsm");
> Cu.import("resource:///modules/devtools/LayoutHelpers.jsm");
>+Cu.import("resource:///modules/devtools/CssLogic.jsm");
> 
> var EXPORTED_SYMBOLS = ["LayoutView"];
> 
>+// Used to split on css line separators
>+const CSS_LINE_RE = /(?:[^;\(]*(?:\([^\)]*?\))?[^;\(]*)*;?/g;
>+
>+// Used to parse a single property line.
>+const CSS_PROP_RE = /\s*([^:\s]*)\s*:\s*(.*?)\s*(?:! (important))?;?$/;
>+

I don't think you need these changes anymore.
Comment 9 Dave Camp (:dcamp) 2012-05-24 13:59:50 PDT
https://hg.mozilla.org/integration/fx-team/rev/4798809c94d6

(I made the changes from Comment 8)
Comment 10 Rob Campbell [:rc] (:robcee) 2012-05-26 10:24:09 PDT
https://hg.mozilla.org/mozilla-central/rev/4798809c94d6

Note You need to log in before you can comment on or make changes to this bug.