709756 - expand/close marks and html elements are not aligned in html inspector

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Leonard Camacho [:lcamacho]]

Attachment: inspector.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111211 Firefox/11.0a1
Build ID: 20111211031118

Steps to reproduce:

On Ubuntu open inspector select a html element that it has others elements inside or look for the <head> element in html view


Actual results:

Look for the expand/close mark on the left is not aligned with the html element and makes difficult to expand/close


Expected results:

Expand/close mark should be aligned with html element

Comment 1

[Paul Rouget [:paul]]

Do you use the default Ubuntu theme?

Comment 2

[Leonard Camacho [:lcamacho]]

Yes, default Ubuntu theme plus new profile without Add-ons or themes installed.

Comment 3

[Leonard Camacho [:lcamacho]]

I have a patch for this but I use a background image of arrows that is used on style inspector instead of -moz-appearance: treetwisty, I don't know if we can use this as a temporary patch?

Comment 4

[Leonard Camacho [:lcamacho]]

Attachment: patch.diff

Use a background image instead of -moz-appearance: treetwisty to make this look good on Ubuntu

Comment 5

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

Comment on attachment 582583 [details] [diff] [review]
patch.diff

can you make this work with the -moz-appearance: treetwisty? We'd like to use that if possible since it is a system-supplied widget.

Also, wondering if your monospace font settings are standard?

I'm not a regular linux user, but I have asked Mihai Sucan for a review round as he may have some opinions.

Comment 6

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

could someone confirm that this is a bug on other linux desktops?

Comment 7

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

Comment on attachment 582583 [details] [diff] [review]
patch.diff

also pinging dao for a feedback round.

lcamacho is on ubuntu 11 (.04 and .10). Looking for linux users to give feedback to see if the images are broken there. The twisties look fine in Ubuntu 10 on my machine.

Comment 8

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

confirming this, though would appreciate some linux testing for real confirmation.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 582583 [details] [diff] [review]
patch.diff

>-  -moz-appearance: treetwisty;
>+  background: url("chrome://browser/skin/devtools/arrows.png") 0 0;

Just like in the style inspector, we should make the native appearance work rather than removing it in favor of custom images.

Comment 10

[Leonard Camacho [:lcamacho]]

Attachment: patch V2

Here's a second try, I check this against the four themes on Ubuntu. BTW without this patch the twisty looks good on these themes HightContrast and HighContratsInverse, just looks bad on Ambiance and Radiance.

I need confirmation if this looks good on another linux distribution.

Comment 11

[Mihai Sucan [:msucan]]

Comment on attachment 582999 [details] [diff] [review]
patch V2

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

I find the alignment to be better without the patch on Ubuntu 10.04 LTS with the Ambiance theme. I use the Clearlooks theme on my system, and with that theme, this patch makes the alignment better.

Given the weird situation, I am giving an f+. Probably we can't have this perfectly aligned for all themes. Anyhow, differences are small enough.

Comment 12

[Paul Rouget [:paul]]

Attachment: patch 1 (paul)

I have been looking at this for the style inspector before. That was my conclusion:

The size of the twisty is 15px (no matter what the theme is). It comes
from the twisty appearance (we can't override this size, the
height/width value are no-op here).

Apparently, most of the GTK themes use a 13 pixel twisty image, aligned on top of the twisty rect.

A work around would be to use top: 2px.

We should investigate and see if this is a general GTK rule.

Comment 13

[Paul Rouget [:paul]]

Also, if I am not mistaken, we use a non-fixed line/font-height. The twisty is absolute positioned, that makes it not vertical-align-able with the text.

On my system, the line-height is 13px, exactly the size of the image of the twisty, so the previous work around works, but if we get something different, that won't work anymore.

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 582999 [details] [diff] [review]
patch V2

Works for me. The whole layout is pretty messy, though. Would be better if we avoided position:absolute.

Comment 15

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/cd03664850ed

Comment 16

[Leonard Camacho [:lcamacho]]

After some research I found that maybe is a Ubuntu theme error if you go to /usr/share/themes/(Radiance|Ambiance)/gtk-2.0/gtkrc and search for GtkTreeView::expander-size this is set to 7 (this change was introduced in Ubuntu maverick that is why if you look on previous releases everything looks good) but GtkExpander::expander-size is set to 11 and I think both should look the same after all both indicate that theres more info inside the element and if you set GtkTreeView::expander-size = 11 plus Paul's patch everything looks good, so I look for the owner of this commit and contact Andrea Cimi at canonical to ask if theres a chance to change this size for the precise release.

But even if this change made to precise pangolin release previous Ubuntu releases are going to look bad, so robcee tells me that maybe the best we can do here is use the patch use arrows.png.

As you can see the size of twisty can be changed very quick and every theme set the size to what ever they want and if they don't the size by default is 12 according to this page http://developer.gnome.org/gtk/stable/GtkTreeView.html#GtkTreeView--s-expander-size

Comment 17

[Ed Morley [:emorley]]

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

Comment 18

[Leonard Camacho [:lcamacho]]

I'm seeing the wrong alignment again, maybe the new style cause this.

Reopening to work on a new patch.

Comment 19

[Paul Rouget [:paul]]

Work should happen there: bug 795176