Closed Bug 1028235 Opened 10 years ago Closed 10 years ago

Additional vertical scrollbar in computed view

Categories

(DevTools :: General, defect)

Product:
DevTools
Component:
General
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Firefox 33

People

(Reporter: elbart, Assigned: bgrins)

References

Details

Attachments

(3 files, 2 obsolete files)

Top before, bottom after the regression-range
180.96 KB, image/png
Details
1028235_comparison_200%.png
13.66 KB, image/png
Details
computed-scrollbars.patch
1.87 KB, patch
pbro
: review+
ntim
: feedback+
Details | Diff | Splinter Review 
In the Inspector, there's an unnecessary scrollbar in the "Computed"-panel, and the label "Browser styles" is lower than the checkbox next to it.

Regression range:
m-c:
Last good revision: 48eee276b1ee (2014-06-13)
First bad revision: 86aa28ce309e (2014-06-14)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=48eee276b1ee&tochange=86aa28ce309e

m-i:
Last good revision: ca5b7ed0dd5a
First bad revision: e9f6e6ec3cde
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ca5b7ed0dd5a&tochange=e9f6e6ec3cde
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
bug 1025057 hasn't fixed the mentioned issues.
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
Blocks: 942292
(In reply to Elbart from comment #2)
> bug 1025057 hasn't fixed the mentioned issues.

The misalignment of the checkbox, "Browser styles" label, and search box is not fixed in Nightly by bug 1025057?
I don't know why, but setting the height to the devtools-toolbar with the search box to 25px instead of 24px removes the secondary scrollbar (which is overflow for the <body> tag.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Additional vertical scrollbar and other layout issues in "Computed" - regression → Additional vertical scrollbar in computed view
(In reply to Brian Grinstead [:bgrins] from comment #3)
> The misalignment of the checkbox, "Browser styles" label, and search box is
> not fixed in Nightly by bug 1025057?

It's not as bad as before, but not yet as good as it was.

Attachment:
 Top: before bug 942292
 Middle: after bug 942292 and before bug 1025057
 Bottom: after bug 1025057

Just noticed that the looking glass is also a bit higher than before.
Attached patch computed-scrollbars.patch (obsolete) — — Splinter Review 
Tim, I believe this resolves the scrollbar issue.  Can you confirm?  And also, does it help with alignment?
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Attachment #8447191 - Flags: feedback?(ntim007)
Comment on attachment 8447191 [details] [diff] [review]
computed-scrollbars.patch

This doesn't fix the scrollbar issue, neither the checkbox issue. 
You might want to increase the toolbar height for now or just set overflow:hidden to the toolbar. Alignment issues can be fixed by decreasing the checkbox's top margin.
Attachment #8447191 - Flags: feedback?(ntim007)
Attached patch computed-scrollbars.patch (obsolete) — — Splinter Review 
How about this one?
Attachment #8447191 - Attachment is obsolete: true
Attachment #8447252 - Flags: feedback?(ntim007)
Comment on attachment 8447252 [details] [diff] [review]
computed-scrollbars.patch

Doesn't seem to work, which is quite strange. It worked when I tested it using an userstyle in an affected build...

Anyways, I investigated further, I noticed that the line-height is set to 24px by the toolbar by default. And that was causing the problem. 
The following tweaks should work  :
- setting line-height: 22px on the toolbar
- setting line-height: 21px on the checkbox
Attachment #8447252 - Flags: feedback?(ntim007)
OK, I think this one may do it.  Does it look good?
Attachment #8447252 - Attachment is obsolete: true
Attachment #8447971 - Flags: feedback?(ntim007)
Comment on attachment 8447971 [details] [diff] [review]
computed-scrollbars.patch

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

This seems identical to the previous one.
Attachment #8447971 - Flags: feedback?(ntim007)
Comment on attachment 8447971 [details] [diff] [review]
computed-scrollbars.patch

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

::: browser/themes/shared/devtools/computedview.css
@@ +151,2 @@
>    width: 100%;
> +  border-bottom-width: 0;

It is different from the previous patch, because it doesn't add back a 1px border to the top (just removes the bottom).  This seems to resolve the issue for me on Windows, but it'd be great if you could confirm.
Comment on attachment 8447971 [details] [diff] [review]
computed-scrollbars.patch

See Comment 12
Attachment #8447971 - Flags: feedback?(ntim007)
Comment on attachment 8447971 [details] [diff] [review]
computed-scrollbars.patch

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

Fixes the issue for me.
Attachment #8447971 - Flags: feedback?(ntim007) → feedback+
Comment on attachment 8447971 [details] [diff] [review]
computed-scrollbars.patch

This is a fix for the computed view in Windows.  Basically removes the bottom border (on all platforms) for the search toolbar at the bottom of the computed view, and overrides a border UA style on the checkbox.
Attachment #8447971 - Flags: review?(pbrosset)
Attachment #8447971 - Flags: review?(pbrosset) → review+
https://hg.mozilla.org/mozilla-central/rev/78eab9f2bf43
Status: ASSIGNED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 33
QA Whiteboard: [good first verify]
[testday-20140912]

Verified on Firefox 33.0 beta 3 on Windows. No additional scroll-bar and the label seems to be aligned with the check-box.
Marking verified fixed based on comment #19.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: