Note: There are a few cases of duplicates in user autocompletion which are being worked on.

enable native theme in HTML content on Linux

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
12 years ago
3 years ago

People

(Reporter: vlad, Assigned: Michael Ventnor)

Tracking

(Depends on: 1 bug)

Trunk
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

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?)
Assignee: nobody → roc
Created attachment 214490 [details] [diff] [review]
disable HTML native theme in gtk2

Disable native theme in HTML agai
Attachment #214490 - Flags: review?(roc)
Attachment #214490 - Flags: superreview+
Attachment #214490 - Flags: review?(roc)
Attachment #214490 - Flags: review+
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 [details] [diff] [review]
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.
Depends on: 333249
Depends on: 333250

Comment 10

11 years ago
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.
Duplicate of this bug: 372079
(Assignee)

Comment 13

10 years ago
Created attachment 266306 [details] [diff] [review]
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.
Attachment #266306 - Flags: superreview?(roc)
Attachment #266306 - Flags: review?(roc)
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...
(Assignee)

Comment 15

10 years ago
(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.
(Assignee)

Comment 17

10 years ago
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.
Attachment #266306 - Attachment is obsolete: true
Attachment #266306 - Flags: superreview?(roc)
Attachment #266306 - Flags: review?(roc)
If you're willing to do your best, that's good enough for me! :-)
(Assignee)

Comment 19

10 years ago
Created attachment 266552 [details] [diff] [review]
Second try

Fix some issues regarding button and dropdown size.
Attachment #266552 - Flags: superreview?(roc)
Attachment #266552 - Flags: review?(roc)
(Assignee)

Comment 20

10 years ago
Created attachment 266555 [details] [diff] [review]
Second patch for real

Sorry, review this one.
Attachment #266552 - Attachment is obsolete: true
Attachment #266555 - Flags: superreview?(roc)
Attachment #266555 - Flags: review?(roc)
Attachment #266552 - Flags: superreview?(roc)
Attachment #266552 - Flags: review?(roc)
Assignee: roc → ventnor.bugzilla
+                *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?
(Assignee)

Updated

10 years ago
Attachment #266555 - Attachment is obsolete: true
Attachment #266555 - Flags: superreview?(roc)
Attachment #266555 - Flags: review?(roc)
(Assignee)

Comment 22

10 years ago
Created attachment 267091 [details] [diff] [review]
Third patch

Fixes review comments. Yes, printing and print preview still work.
Attachment #267091 - Flags: superreview?(roc)
Attachment #267091 - Flags: review?(roc)
Comment on attachment 267091 [details] [diff] [review]
Third patch

okay, let's go!
Attachment #267091 - Flags: superreview?(roc)
Attachment #267091 - Flags: superreview+
Attachment #267091 - Flags: review?(roc)
Attachment #267091 - Flags: review+
(Assignee)

Comment 24

10 years ago
(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 ;)
(Assignee)

Updated

10 years ago
Whiteboard: [checkin needed]

Comment 25

10 years ago
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! :)
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]

Comment 26

10 years ago
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?
(Assignee)

Comment 27

10 years ago
(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.

(Assignee)

Updated

10 years ago
Duplicate of this bug: 232553

Comment 29

10 years ago
Created attachment 267373 [details] [diff] [review]
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.
Attachment #267373 - Flags: superreview?(roc)
Attachment #267373 - Flags: review?(roc)
(Assignee)

Comment 30

10 years ago
(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?

Comment 32

10 years ago
(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.
(Assignee)

Comment 33

10 years ago
(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 [details] [diff] [review]
Enable native rendering for multiple selects

okay then...
Attachment #267373 - Flags: superreview?(roc)
Attachment #267373 - Flags: superreview+
Attachment #267373 - Flags: review?(roc)
Attachment #267373 - Flags: review+

Comment 35

10 years ago
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.

Updated

10 years ago
Depends on: 383479

Updated

10 years ago
Depends on: 383503
(Assignee)

Updated

10 years ago
No longer depends on: 383503
(Assignee)

Comment 36

10 years ago
Vassil's followup hasn't been checked in yet, adding smoke signal to whiteboard.
Whiteboard: [checkin needed]

Updated

10 years ago
Depends on: 383583

Comment 37

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

Comment 38

10 years ago
(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

10 years ago
Created attachment 267653 [details]
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.
(Assignee)

Comment 40

10 years ago
Confirmed in _some_ lists (I can't reproduce this on the CC list above).
I landed attachment 267373 [details] [diff] [review] on the trunk.
Whiteboard: [checkin needed]

Updated

10 years ago
Depends on: 384266

Comment 42

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

Updated

10 years ago
Depends on: 385686

Comment 43

10 years ago
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.

Updated

10 years ago
Depends on: 384112

Updated

10 years ago
Depends on: 388131
(Assignee)

Updated

10 years ago
Depends on: 390719
(Assignee)

Updated

10 years ago
Depends on: 393737

Updated

10 years ago
Depends on: 402940

Comment 44

10 years ago
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?
(Assignee)

Comment 45

10 years ago
(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

10 years ago
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

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

Comment 48

10 years ago
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

10 years ago
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.

Updated

7 years ago
You need to log in before you can comment on or make changes to this bug.