412092 - GTK: List and Tree items should be painted natively

Status: RESOLVED WONTFIX

(Core :: Widget: Gtk, defect)

Description

[u294409]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; pl; rv:1.9b3pre) Gecko/2008011204 openSUSE/10.3 Minefield/3.0b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; pl; rv:1.9b3pre) Gecko/2008011204 openSUSE/10.3 Minefield/3.0b3pre

firefox shouldn't draw own gradient on addons-manager items.
they should look like tree elements in gtk/menu items in gtk.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Actual Results:  
firefox draws gradient on items.

Expected Results:  
native look for items.

Comment 1

[Steve England [:stevee]]

please do not request blocking for bugs that are not confirmed.

Comment 2

[u294409]

ping

Comment 3

[Ian Spence]

Renaming and taking this because for the life of me, I can't find another bug that has to do with tree items. (If I created a bug for tree items, this would be marked as a duplicate)

Gonna try and update it to Core: Widget : GTK

Comment 4

[Ian Spence]

Attachment: Example of the work done

Well I have an update.

I currently have the painting work done, as can be seen in the attachment.

And now on to the bad news...

I can see how to handle painting within actual trees.  Listboxes are no problem, but trees are weird.  Apparently they do not handle -moz-appearance at all, so I'm stuck using .

That being said, I should be able to at least give trees better background colors (by introducing new -moz colors).

So any input on how to handle trees would be much appreciated

Comment 5

[Michael Monreal [:micmon]]

The "native" tree element look would also be great for the selection in the Preferences/Addons/PageInfo headers.

Comment 6

[u294409]

Yay, you rock :) !

Comment 7

[u294409]

Does this work for addons manager too?
It draws ugly backgrounds (bug #406883)...

Comment 8

[u294409]

Wrong bug number. This bug was about addons manager :> .

Comment 9

[Ian Spence]

This would be a general fix. It already fixes addons/themes/downloads/applications. It still needs be updated for the error console.

Comment 10

[Ian Spence]

I know there is a reason, but I don't know what it is, so someone please enlighten me.

Currently we have the following CSS.  Thanks to my 
---------------------------------------
listbox {
  -moz-appearance: listbox;
  margin: 2px 4px;
  border: 2px solid;
  -moz-border-top-colors: ThreeDShadow ThreeDDarkShadow;
  -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
  -moz-border-bottom-colors: ThreeDHighlight ThreeDLightShadow;
  -moz-border-left-colors: ThreeDShadow ThreeDDarkShadow;
  background-color: -moz-Field;
  color: -moz-FieldText;
}

listbox[disabled="true"] {
  color: GrayText;
}

/* ::::: listitem ::::: */

listitem {
  -moz-appearance: treeitem;
  color: -moz-TreeItemText;
}

listbox:focus > listitem {
  -moz-appearance: treeitem-focused;
  color: -moz-TreeItemActiveText;
}

listbox:focus > listitem[selected="true"][current="true"] {
  outline: 1px dotted #F3D982;
}

listbox:focus > listitem[current="true"] {
  outline: 1px dotted Highlight;
  -moz-outline-offset: -1px;
}

listitem[selected="true"] {
  background-color: -moz-Dialog;
  color: -moz-DialogText;
}

listbox:focus > listitem[selected="true"] {
  background-color: Highlight;
  color: HighlightText;
}
---------------------------------------

Now, in a perfect world (ie, a fully -moz-appearance world), the CSS could be
---------------------------------------
listbox {
  -moz-appearance: listbox;
  margin: 2px 4px;
}

listbox[disabled="true"] {
  color: GrayText;
}

/* ::::: listitem ::::: */

listitem {
  -moz-appearance: treeitem;
  color: -moz-TreeItemText;
}

listbox:focus > listitem {
  -moz-appearance: treeitem-focused;
  color: -moz-TreeItemActiveText;
}
---------------------------------------

So are the extra selectors, etc for backwards compatibility?

Comment 11

[Teune van Steeg]

There are:
  - focused listboxes with:
     - selected items
     - selected items which are focused (current)
     - unselected items
     - unselected items which are focused (current)
  - unfocused listboxes with:
     - selected items
     - unselected items.

Therefore are those extra selectors

   

Comment 12

[Ian Spence]

I messed up the last part of my question (which happens to be the question part)

I have all of those states handled in the painting code.

I'm curious as to why we still have background-color definitions when -moz-appearance is going to override it anyway?

Comment 13

[Teune van Steeg]

(In reply to comment #12)
> I'm curious as to why we still have background-color definitions when
> -moz-appearance is going to override it anyway?
> 

Ah OK: that can just disappear (see bug 412307)

Comment 15

[u294409]

Ian, does with fix flat comboboxes menus in websites?

Comment 16

[Ian Spence]

No, but you'll be happy to know that it apparently fixes the giant autocomplete dropdown that appears when you start typing in a url

Comment 17

[u294409]

Fixes autocomplete? Yay! You rock :) .

I am really happy :) .

Comment 18

[Ian Spence]

Attachment: WIP - Implement pretty list items

Posting a WIP so that I can get some feedback while working on this bug. This patch touches a lot of different parts of the code (from javascript to gtk to adding colors)

I'm working on standardizing the alternating colors of listitems by taking the download managers approach (since that is the part I was most afraid of touching) and applying it to the extensions dialog.  I'll use that code to style the error console, magic url bar, and applications pane later on.

Comment 19

[Ian Spence]

Attachment: How the new autocomplete currently looks

Comment 20

[Teune van Steeg]

Wouldn't it be possible in nsNativeThemeGTK.cpp to set stateFrame to aFrame.GetParent() and get the focused state from that, instead of using NS_THEME_TREEVIEW_TREEITEM_FOCUSED?

In this patch you only use aState to get the disabled state, but I suppose that can also be gotten from the parent. Or are there situations where individual cells get disabled?

Comment 21

[u294409]

Lovely autocomplete... Looks nice :) .

Comment 22

[Ian Spence]

The problem with using the parent state is that I don't always know what the parent is. And sometimes I need to paint an item as focused even though it lacks focus, like the autocomplete and error console.

So while I can't get focus from the parent, you are correct that I should be getting disabled from the parent

Comment 23

[Grey Nicholson (they/them)]

Is the auto-complete's green text colour still hard-coded? It's unreadable in dark themes.

Comment 24

[Ian Spence]

Yes it is. I don't know why they added a color for that text, so I don't think I should be removing it from the code

Comment 25

[Michael Monreal [:micmon]]

Not really hardcoded... I use this personally:

.ac-comment { font-size: 10pt !important; }
.ac-url-text { color: #4e9a06 !important; }
.ac-url-text[selected] { color: inherit !important; }

Comment 26

[Ian Spence]

Attachment: Updated patch

This includes a lot of additions to the previous patch.

I'll still need to attach the following files:
/toolkit/themes/gnomestripe/richlistbox.css
/toolkit/themes/gnomestripe/console/console.css

My main problem is that I replicate stripeifyList in a few files.  I don't know if there is a global js file I could add it to and remove it from all other scripts

Comment 27

[Ian Spence]

Wanted to get the opinions of some developer people.

My current worry is about performance.  It turns out that treechildren:-moz-tree-row does allow -moz-appearance (at least according to http://developer.mozilla.org/en/docs/CSS::-moz-tree-row )

My issue is that if I did implement that, I would hit a huge performance issue.  I would be painting ~30 tree items. Since treechildren is a single frame, I believe I would repaint every single tree item whenever any of them changed.

My planned solution is the following:
Only give selected tree items a -moz-appearance attribute.  The rest of the items would just use a base background color. This works because treeitems default to the background of the tree itself.  My only issue then would be that padding would be inconsistent. So my plan is to not support any widget padding/borders/etc in nativeThemeGtk.cpp. All padding would be done by CSS, thus preventing -moz-appearance from changing anything other than background. So with this, in treechildren I would only paint one item per tree, but no user could tell the difference in anything other than performance.

The issue with this is that -moz-tree-row doesn't have normal states, so I'd need to work around that.  My current idea is to add new NS_THEME_TREEITEM_* options.

NS_THEME_TREEITEM: This is the default option for unfocused listboxes

NS_THEME_TREEITEM_FOCUSED: This is the default option for focused listboxes and elements that have parents that don't recieve focus (autocomplete) or where the focus doesn't matter

NS_THEME_TREEITEM_SELECTED: This is only for -moz-tree-row(selected).  Paints the same as NS_THEME_TREEITEM when !state->focused

NS_THEME_TREEITEM_SELECTED_FOCUSED: This is only for -moz-tree-row(selected).  Paints the same as NS_THEME_TREEITEM when state->focused

These additions really only add some complexity to the CSS code, since you'll need to know which -moz-appearance to use. That being said, I think this would be the best way to move.  This would eventually allow us to use NS_THEME_TREEITEM_SELECTED_FOCUSED for the menus that result from combobox activations.

Anyhoo, wanted the opinion of people who know more about this stuff than I do.

Also, if anyone could tell me if there is a javascript file that is used globally, that would be great so I didn't need to keep redefining that function.


So now that you've made it through that post, I'm looking for input on:
1) The use of 4 different NS_THEME_* declarations
2) Not using xthickness and ythickess (or anything GTK-ish) in deciding on padding for the items
3) That global javascript file

Comment 28

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sounds reasonable, but might be too invasive at this late stage of FF3. We could still use a patch for post-FF3.

Comment 29

[u294409]

> might be too invasive at this late stage of FF3

So you should care earlier...

I think this should be fixed as fast possible.

Comment 30

[u294409]

Attachment: ugly border around combobox' popup menu

Comment 31

[Ian Spence]

Unless you have magically become competent in coding, this won't be fixed for Firefox 3

Comment 34

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:stransky, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 35

[Martin Stránský [:stransky] (ni? me)]

Not relevant any more.