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: PTO - 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, PTO - 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, PTO - 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 User image 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 User image PTO - 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 User image PTO - 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 User image 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 User image PTO - 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 User image 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 User image PTO - 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 User image 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 User image 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 User image Rob Campbell [:rc] (:robcee) 2011-11-15 14:08:27 PST
https://hg.mozilla.org/integration/fx-team/rev/e2aca39ef669
Comment 10 User image 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 User image 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 User image Dave Camp (:dcamp) 2011-11-15 16:48:31 PST
Followup landed https://hg.mozilla.org/integration/fx-team/rev/3831a1444526
Comment 14 User image 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 User image 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 User image Rob Campbell [:rc] (:robcee) 2011-11-30 06:12:56 PST
aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/b1b8ea656961
Comment 17 User image 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.