329846 - enable native theme in HTML content on Linux

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

With roc's work with the native theme renderer, we can now do native theme rendering in content (we can print it, etc.).  But there are a few problems that need to be fixed before we can turn it on by default.

Things that I noticed:

- 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
- dropdown boxes look very wrong, because we're making them up from elements (e.g. buttons and boxes) as opposed to using a native dropdown box
- focus rectangles get drawn twice, once by the native theme, and once by us
- checkboxes and radio buttons get clipped on the right (by a pixel?)

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: disable HTML native theme in gtk2

Disable native theme in HTML agai

Comment 2

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

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.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 4

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

Attachment: Disable Mozilla focus rect if the native theme paints focus

Comment 5

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

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.

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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

Comment 7

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

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

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(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?

Comment 9

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

> - 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.

Comment 10

[Mikko Virkkilä]

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.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 13

[Michael Ventnor]

Attachment: 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.

Comment 14

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

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...

Comment 15

[Michael Ventnor]

(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)

Comment 16

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

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 17

[Michael Ventnor]

Comment on attachment 266306 [details] [diff] [review]
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.

Comment 18

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

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

Comment 19

[Michael Ventnor]

Attachment: Second try

Fix some issues regarding button and dropdown size.

Comment 20

[Michael Ventnor]

Attachment: Second patch for real

Sorry, review this one.

Comment 21

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

+                *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?

Comment 22

[Michael Ventnor]

Attachment: Third patch

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

Comment 23

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

Comment on attachment 267091 [details] [diff] [review]
Third patch

okay, let's go!

Comment 24

[Michael Ventnor]

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

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

Comment 25

[Michael Wu [:mwu]]

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! :)

Comment 26

[Jeff Fortin]

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?

Comment 27

[Michael Ventnor]

(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.

Comment 29

[Vassil Stefanov]

Attachment: 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.

Comment 30

[Michael Ventnor]

(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.

Comment 31

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

Is MOZ_GTK_ENTRY really the right thing for listboxes?

Comment 32

[Vassil Stefanov]

(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.

Comment 33

[Michael Ventnor]

(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 34

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

Comment on attachment 267373 [details] [diff] [review]
Enable native rendering for multiple selects

okay then...

Comment 35

[Xavier Chantry]

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.

Comment 36

[Michael Ventnor]

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

Comment 37

[Michael Wu [:mwu]]

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

Comment 38

[Vassil Stefanov]

(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.

Comment 39

[Michael Wu [:mwu]]

Attachment: 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.

Comment 40

[Michael Ventnor]

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

Comment 41

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I landed attachment 267373 [details] [diff] [review] on the trunk.

Comment 42

[Boris Zbarsky [:bzbarsky]]

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

Comment 43

[pantsgolem]

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.

Comment 44

[Jan Niklas Hasse]

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?

Comment 45

[Michael Ventnor]

(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.

Comment 46

[David Muir]

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.

Comment 47

[Jan Niklas Hasse]

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

Comment 48

[David Muir]

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".

Comment 49

[Andrea Cimitan]

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.