427979 - Bring back download manager banding, this time with native colours

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Michael Ventnor]

Attachment: Patch

Linux was an unfortunate innocent victim of the decision to remove banding from the download manager on Windows. I contacted faaborg on IRC and to paraphrase him, if you're going to do it, do it right.

I _finally_ found out what makes treeview backgrounds tick. Its very slightly uglier than Mac, but its trivial enough that it can and should make Firefox 3. Considering that a lot of this code mimics GTK's behaviour exactly, the risk is minimal (unless the GTK developers have had risky code for ages!).

Not much in the way of tests for this bug, other than open up the download manager and verify the bands it produces matches what your GTK theme does for regular treeviews.

Comment 1

[Michael Ventnor]

Attachment: Patch 1.001

Dunno how a tab managed to find its way in there...

Comment 2

[Michael Ventnor]

Attachment: Patch 2

Now with less duplication!

Comment 3

[Julien "_FrnchFrgg_" RIVAUD]

Comment on attachment 314590 [details] [diff] [review]
Patch 1.001

r+, provided you change the code in InitLookAndFeel to code like

GdkColor* colorValue = NULL;
get(colorValue);
if (!colorValue)
{
 getanother(colorValue)
 if (!colorValue)
   colorValue = new_color_value(base)

}
sOddCellBackground = ...
gdk_color_free(...)

Comment 4

[Julien "_FrnchFrgg_" RIVAUD]

Comment on attachment 314736 [details] [diff] [review]
Patch 2

Ok, this way works too.

Comment 5

[Michael Ventnor]

Comment on attachment 314736 [details] [diff] [review]
Patch 2

Its a small patch that restores banding for Linux (giving it parity with Mac) while making it take on completely native colours at the same time. By doing this the DL manager looks like a native list like it was meant to (and is, on Mac).

The code shouldn't be risky, it's almost exactly what GTK does to draw odd rows.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 314736 [details] [diff] [review]
Patch 2

a1.9=beltzner

Comment 7

[Wolfgang Rosenauer [:wolfiR]]

Checking in widget/src/gtk2/nsLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsLookAndFeel.cpp,v  <--  nsLookAndFeel.cpp
new revision: 1.39; previous revision: 1.38
done
Checking in widget/src/gtk2/nsLookAndFeel.h;
/cvsroot/mozilla/widget/src/gtk2/nsLookAndFeel.h,v  <--  nsLookAndFeel.h
new revision: 1.9; previous revision: 1.8
done
Checking in toolkit/themes/gnomestripe/mozapps/downloads/downloads.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/mozapps/downloads/downloads.css,v  <--  downloads.css
new revision: 1.9; previous revision: 1.8
done

Comment 8

[Ian Spence]

I'm curious about why we didn't implement the even color as well.  While I understand that assuming the even color is the same as -moz-Field is safe, it's not correct.  There are always chances that they are different