Last Comment Bug 700770 - Style Inspector's Rule View can't scroll vertically
: Style Inspector's Rule View can't scroll vertically
Status: RESOLVED FIXED
[styleinspector][ruleview][qa+][qa!:10]
: verified-beta
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: All All
: P2 major with 1 vote (vote)
: Firefox 11
Assigned To: Michael Ratcliffe [:miker] [:mratcliffe]
:
: J. Ryan Stinnett [:jryans] (use ni?)
Mentors:
Depends on: 701212
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-08 12:33 PST by Jose Fandos
Modified: 2012-01-03 13:12 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified


Attachments
Flexbox plus margin (2.90 KB, patch)
2011-11-09 01:31 PST, Michael Ratcliffe [:miker] [:mratcliffe]
dao+bmo: review-
Details | Diff | Splinter Review
Flexbox plus margin 2 (3.43 KB, patch)
2011-11-14 07:23 PST, Michael Ratcliffe [:miker] [:mratcliffe]
dao+bmo: review+
rcampbell: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
small followup (1.06 KB, patch)
2011-11-15 16:41 PST, Dave Camp (:dcamp)
rcampbell: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Jose Fandos 2011-11-08 12:33:24 PST
1. On this page, right click any element and choose "Inspect Element".
2. Select "Style" on the bottom right to get the CSS sidebar.
3. Try scrolling vertically. It doesn't. If the content doesn't take more space than available vertically, try resizing the browser window to make it so, and try to scroll then.
Comment 1 Michael Ratcliffe [:miker] [:mratcliffe] 2011-11-09 01:06:07 PST
This is bad ... there is no vertical scrollbar in the Rule View. I will take a look.
Comment 2 Michael Ratcliffe [:miker] [:mratcliffe] 2011-11-09 01:31:00 PST
Created attachment 573140 [details] [diff] [review]
Flexbox plus margin

Should be a very easy review because we have only changed a div to a hbox, set flex to 1 and given it overflow.

I have also added 5px padding on the sides of the rules to make the overall view look better.
Comment 3 Dão Gottwald [:dao] 2011-11-09 05:12:54 PST
As far as I remember, this document has a content stylesheet -- overflow:auto belongs there.
Comment 4 Michael Ratcliffe [:miker] [:mratcliffe] 2011-11-14 07:23:52 PST
Created attachment 574295 [details] [diff] [review]
Flexbox plus margin 2

Addressed reviewers comment
Comment 5 Rob Campbell [:rc] (:robcee) 2011-11-14 12:31:51 PST
Comment on attachment 574295 [details] [diff] [review]
Flexbox plus margin 2

-  this.element = this.doc.createElementNS(HTML_NS, "div");
+  this.element = this.doc.createElementNS(XUL_NS, "vbox");
   this.element.setAttribute("tabindex", "0");
   this.element.classList.add("ruleview");
+  this.element.flex = 1;

why is this needed?
Comment 6 Michael Ratcliffe [:miker] [:mratcliffe] 2011-11-15 02:34:55 PST
Because the style view contents are inside a xul:window. Using height: 100% on a div will not work because there is no reference value (xul uses flex). In fact, height: 1% will still be 100% of the body of the document so there would still not be a scrollbar.

We could also have kept the element as a div and added:
  display: -moz-box;
  -moz-box-orient: vertical;
  -moz-box-flex: 1;

But that requires slightly more markup and seems pointless when there is a xul element designed to do exactly the same job.
Comment 7 Rob Campbell [:rc] (:robcee) 2011-11-15 09:59:31 PST
Comment on attachment 574295 [details] [diff] [review]
Flexbox plus margin 2

would like to get this into aurora as well as it is pretty broken without it.
Comment 8 Rob Campbell [:rc] (:robcee) 2011-11-15 14:05:44 PST
Comment on attachment 574295 [details] [diff] [review]
Flexbox plus margin 2

wfm, Mike!
Comment 9 Rob Campbell [:rc] (:robcee) 2011-11-15 14:08:27 PST
https://hg.mozilla.org/integration/fx-team/rev/e2aca39ef669
Comment 10 Asa Dotzler [:asa] 2011-11-15 14:51:22 PST
Comment on attachment 574295 [details] [diff] [review]
Flexbox plus margin 2

Please land on m-c before requesting approval. Thanks.
Comment 11 Dave Camp (:dcamp) 2011-11-15 16:41:11 PST
Created attachment 574729 [details] [diff] [review]
small followup

This broke the relative positioning we use for measuring text.  Followup moves the relative positioning down to a rule element (we just need something to position against absolutely, the thing we position isn't visible).
Comment 12 Dave Camp (:dcamp) 2011-11-15 16:48:31 PST
Followup landed https://hg.mozilla.org/integration/fx-team/rev/3831a1444526
Comment 14 Dave Camp (:dcamp) 2011-11-18 11:22:39 PST
Comment on attachment 574729 [details] [diff] [review]
small followup

Small styling change and a followup, landed on fx-team and m-c.
Comment 15 Alex Keybl [:akeybl] 2011-11-22 14:57:17 PST
Comment on attachment 574295 [details] [diff] [review]
Flexbox plus margin 2

[Triage Comment]
This fix is related to a major new upcoming feature, so we'll it. Please land on Aurora as soon as possible.
Comment 16 Rob Campbell [:rc] (:robcee) 2011-11-30 06:12:56 PST
aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/b1b8ea656961
Comment 17 Paul Silaghi, QA [:pauly] 2011-12-27 07:49:46 PST
The scroll seems to work fine now. The bug is verified fixed on Firefox 10 beta1:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (X11; Linux x86_64; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0

But I noticed one thing. Go to www.yahoo.com and click to Inspect Element on the right empty side of the page. A black screen is seen for a very short time. 
What do you think?

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