Widgets should be UbuntuLooks

Created attachment 214490
disable HTML native theme in gtk2

Disable native theme in HTML agai

Clearlooks paints buttons and textboxes with a gradient border, like so:

  cl_rectangle_set_gradient (&r.border_gradient,
                             &clwindowstyle->inset_dark[windowstate],
                             &clwindowstyle->inset_light[windowstate]);

It obtains clwindowstyle by peeking at the widget's parent, which we control, but that doesn't really help. There is no way to make this style look good on a non-uniform background; the theme engine needs to be redesigned to be useful for Web content. Since the Clearlooks theme is the basis for Novell's next theme, my motivation to get this working just dropped to zero.

However, I've already fixed the issue with multiple focus rects, so I'll attach that patch here for posterity.

One thing that we can do is to get Novell and whomever else to develop a separate firefox theme that looks identical to Clearlooks or whatever. We can put in some code to do auto-theme selection. A native firefox theme should be far faster than calling in to gtk to do this.

Created attachment 217234
Disable Mozilla focus rect if the native theme paints focus

It couldn't really look identical to Clearlooks, without looking bad on unpredictable backgrounds. This needs to go all the way back up the chain.

Why not? Let's assume for a second that we have border-image support.

Because Clearlooks looks bad if you use it on unpredictable backgrounds :-).

(In reply to comment #7)
> Because Clearlooks looks bad if you use it on unpredictable backgrounds :-).

But I'm saying that it can look almost exactly like Clearlooks, without the background brokenness -- that is, it can do things like style the menu bar/toolbar/tabbar with an appropriate gradient-y background, and just draw buttons and the like by just drawing the button portion, with transparent bits over where the background is. This should work against any background, no? Maybe I'm not understanding what the problem is... can you do a zoomed-in screenshot or something?

> - themes with widgets that have rounded borders (e.g. buttons, text entry
> boxes in Clearlooks) seem to render the default gtk background color around
> their edges

What you're seeing is actually a gradient border (only 3px wide in Novell's Gilouche theme, so it doesn't look very gradient-like to me) drawn by the buttons and text entry boxes themselves. It depend on the button/textbox geometry so we can't pretend it's just part of the parent widget. The gradient is designed based on the assumption that the background was also drawn by the theme. If I tweak the theme border width to 2px, it turns off the gradient and looks OK, but of course that's a fairly significant visual change.

I guess I could imagine reinterpreting the theme to use a gradient with varying alpha so that you get approximately the right results when drawn onto the theme background color, and a smooth blend onto any other background. Nevertheless this needs to be discussed with the theme designers to see how they think their theme should behave for HTML given they can't make assumptions about what the background looks like ... it's a design constraint that they obviously didn't have in mind before, and something they need to think about.

I'm sure there is some obvious reason why this won't work, but:

Ok, if I've understood the basics correctly the widgets first get drawn to an X buffer by the native theme. Then some magic is done to extract transparency (painting on black and then white). Then this transparent version is copied on to the cairo surface, which is shown.

How about first rendering the page to the point where the background of the widget is known. Then copying that background to the X buffer and having the theme code draw the widget on to that. When the drawing is done, the box can again be copied back to the original cairo surface.

The background itself is irrelevant; some themes simply assume that they were the ones that painted the background, and as such assume they know what the background is. So, they paint things like rounded buttons not with actual round corners, but with sharp corners with a chunk of what they assume is the backgruond behind it.

*** Bug 372079 has been marked as a duplicate of this bug. ***

Created attachment 266306
Turn it back on

Thanks largely to Josh's work for Mac controls, most of the issues are now far easier to fix. I would like to get this in Alpha 6.

Have you verified that Web pages look OK with common themes? I.e. that the issues discussed in this bug are actually fixed? Last I checked the problems are not Firefox problems but in the design of the themes themselves...

(In reply to comment #14)
> Have you verified that Web pages look OK with common themes? I.e. that the
> issues discussed in this bug are actually fixed? Last I checked the problems
> are not Firefox problems but in the design of the themes themselves...
>

That's only the first point, and yes I couldn't find a way around that either. Points 2 and 3 are fixed by changing a few booleans not implemented when this bug was created. I don't see point 4 at all.

I used Ubuntu 7.04 with every theme that came with it and it appears to work very well in terms of cost vs benefit (cost being a few rogue pixels, I don't think that is significant compared with the benefit of native widgets)

OK, good. Are you willing to take ownership of any remaining blocker issues or regressions that come up as a result of turning this on? If you are, we can take this patch.

Comment on attachment 266306
Turn it back on

I'm just fixing a couple of issues regarding size, I should have a new patch soon. I'll do my best at the regressions, but if I can't find a way to fix one can you back this out for me?

Interestingly, my combo boxes have rounded corners yet they don't show the rogue pixel problem.

If you're willing to do your best, that's good enough for me! :-)

Created attachment 266552
Second try

Fix some issues regarding button and dropdown size.

Created attachment 266555
Second patch for real

Sorry, review this one.

+ *right = *left += focus_width + focus_pad + child_spacing;
+ *bottom = *top += focus_width + focus_pad + child_spacing;

I don't like mixing assignment types in the same statement ... make these separate statements.

+ *right = *left += gButtonWidget->style->xthickness;
+ *bottom = *top += gButtonWidget->style->ythickness;

ditto

+ *right = *left += (focus_width + focus_pad);
+ *bottom = *top += (focus_width + focus_pad);

ditto

Otherwise looks good.

It's too late for a change like this to get into alpha5. We could check a revised patch into alpha6 though.

BTW can you verify that printing/print preview still work with this patch?

Created attachment 267091
Third patch

Fixes review comments. Yes, printing and print preview still work.

Comment on attachment 267091
Third patch

okay, let's go!

(In reply to comment #23)
> (From update of attachment 267091 [details])
> okay, let's go!
>

Not me, I don't have CVS rights ;)

Checking in widget/src/gtk2/gtk2drawing.c;
/cvsroot/mozilla/widget/src/gtk2/gtk2drawing.c,v <-- gtk2drawing.c
new revision: 1.23; previous revision: 1.22
done
Checking in widget/src/gtk2/gtkdrawing.h;
/cvsroot/mozilla/widget/src/gtk2/gtkdrawing.h,v <-- gtkdrawing.h
new revision: 1.30; previous revision: 1.29
done
Checking in widget/src/gtk2/nsNativeThemeGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v <-- nsNativeThemeGTK.cpp
new revision: 1.91; previous revision: 1.90
done

Nice! :)

I just wanted to express my thanks. You folks made my day, I've been waiting for these native widgets for over two years. I really hope they stay in the final release :)

By the way, isn't this the same as bug #232553 ?

Also, I guess this mean derivative browsers such as Epiphany get the issue fixed automatically for them too, right?

(In reply to comment #26)
> I just wanted to express my thanks. You folks made my day, I've been waiting
> for these native widgets for over two years. I really hope they stay in the
> final release :)

You're welcome. They'll stay as long as there are no blocker issues that no-one knows how to fix. Are you testing a nightly build to see if everything is working for you?

> By the way, isn't this the same as bug #232553 ?

Yes it is. The whiteboard of that bug even says "DUPEME" :P

> Also, I guess this mean derivative browsers such as Epiphany get the issue
> fixed automatically for them too, right?

They will if they use official Mozilla code from the /widget directory, and not their own implementation.

*** Bug 232553 has been marked as a duplicate of this bug. ***

Created attachment 267373
Enable native rendering for multiple selects

Selects that have multiple set are not styled natively, which may be on purpose or just forgotten. Anyway, this patch seems to enable their native theming. I don't know the code at all so it may be totally wrong, but it works. There is a minor issue with the position of the scrollbar which is too far from the border, but overall it looks better with this patch.

(In reply to comment #29)
> Created an attachment (id=267373) [details]
> Enable native rendering for multiple selects
>
> Selects that have multiple set are not styled natively, which may be on purpose
> or just forgotten. Anyway, this patch seems to enable their native theming. I
> don't know the code at all so it may be totally wrong, but it works. There is a
> minor issue with the position of the scrollbar which is too far from the
> border, but overall it looks better with this patch.

It was forgotten, mea culpa. The scrollbar gave a native illusion. :)
As for the scrollbar position issue described above, I applied the patch and it doesn't look any different to the native textareas. Its not really a problem at all.

Is MOZ_GTK_ENTRY really the right thing for listboxes?

(In reply to comment #31)
> Is MOZ_GTK_ENTRY really the right thing for listboxes?
>

I didn't find a better one so that is why I used that. It looks the way it should, but it may be the wrong type, I don't know.

(In reply to comment #31)
> Is MOZ_GTK_ENTRY really the right thing for listboxes?

What do we do to construct generic GFX listboxes? Running an Ubuntu box I second this, it looks perfect with an entry.

Comment on attachment 267373
Enable native rendering for multiple selects

okay then...

It looks like this have caused Bug 192896 (except that it's now specific to linux, while it was specific to windows back then).
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a6pre) Gecko/20070605 Minefield/3.0a6pre
(same for 20070606 build)

Sorry if this issue is already known, or if it should be reported in a new bug instead of here.

Vassil's followup hasn't been checked in yet, adding smoke signal to whiteboard.

The border around a scrollbar in a list seems to light up on mouseover with this follow up patch.

(In reply to comment #37)
> The border around a scrollbar in a list seems to light up on mouseover with
> this follow up patch.
>

I don't see this. There is a black line left of the scrollbar thumb, but that has been there for a long time and is present on any scrollbar.

Created attachment 267653
Line above scrollbar on hover

Here's the problem. The theme is clearlooks but I can reproduce it on other themes. The top one is when my cursor was hovering above the scrollbar and the bottom one is when it's off the scrollbar. Note the white line above the scrollbar in the top one compared to the bottom one.

Confirmed in _some_ lists (I can't reproduce this on the CC list above).

I landed attachment 267373 on the trunk.

This doesn't seem to play very nice with canvas... See bug 384266.

This part of the patch: https://bugzilla.mozilla.org/attachment.cgi?id=267091&action=diff#mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp_sec7

causes bug 384112. Uncommenting that line fixes the problem.

Hi there! I just love the new native widgets.
But i got one question, concerning the rounded borders. I'm sorry if it has already been answered in any way, my english isn't that good.
I have uploaded two files:
http://watteimdocht.de/jan-nik/works.png
http://watteimdocht.de/jan-nik/doesnt.png
Why do the rounded borders work in the first example (dropdown) but don't look nice with a button?

(In reply to comment #44)
> Hi there! I just love the new native widgets.
> But i got one question, concerning the rounded borders. I'm sorry if it has
> already been answered in any way, my english isn't that good.
> I have uploaded two files:
> http://watteimdocht.de/jan-nik/works.png
> http://watteimdocht.de/jan-nik/doesnt.png
> Why do the rounded borders work in the first example (dropdown) but don't look
> nice with a button?

Clearlooks provided a workaround for a bad bug with buttons in Firefox 1.5. Ever since the interface switched to what it is now in 2.0 and 3.0, the hack is not necessary but was forgotten about, so it stayed. This hack is what causes the bad borders.

The sudden switch to native widgets came as a shock to GTK engine authors everywhere who kept the original Clearlooks Firefox hack. Nowadays things are getting better, the Glossy theme in Ubuntu Gutsy doesn't show the problem but there is still an issue with textbox borders. Hopefully they can take care of any remaining issues before the Fx3 release.

The bug is still there in FF beta 3 (running on Gutsy).
The only themes that seems to work well are Industrial and Gray.
Clearlooks and Human both have the nasty border, while LegacyHuman's buttons look fine, but input boxes have a border.

As Michael Ventnor said: It's not a bug of FF, it's a bug in the GTK engines / the themes.

So we just have to wait for the themes to be fixed?
What about all this talk about FF patches that seem to override the display problem? Or am I totally not understanding half of the posts here?

I know it's a bit of the old "aint my problem" thing, but unless there's some way to override it (which is transparent to the user, i.e. automagically done by FF), people are going to say "ugh, there's something wrong with the buttons in FF".

I fixed that bug in the Murrine engine (svn, not stable release yet), clearlooks seems to work well too.
It's a bug in the engine.