Closed
Bug 329846
Opened 19 years ago
Closed 18 years ago
enable native theme in HTML content on Linux
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: vlad, Assigned: ventnor.bugzilla)
References
(Depends on 1 open bug)
Details
Attachments
(5 files, 3 obsolete files)
1.12 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
6.73 KB,
patch
|
Details | Diff | Splinter Review | |
18.81 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
1.74 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
9.34 KB,
image/png
|
Details |
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
Reporter | ||
Comment 1•19 years ago
|
||
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.
Reporter | ||
Comment 3•19 years ago
|
||
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.
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.
Reporter | ||
Comment 6•19 years ago
|
||
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 :-).
Reporter | ||
Comment 8•19 years ago
|
||
(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•19 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.
Reporter | ||
Comment 11•19 years ago
|
||
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.
Assignee | ||
Comment 13•18 years ago
|
||
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•18 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•18 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•18 years ago
|
||
Fix some issues regarding button and dropdown size.
Attachment #266552 -
Flags: superreview?(roc)
Attachment #266552 -
Flags: review?(roc)
Assignee | ||
Comment 20•18 years ago
|
||
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•18 years ago
|
Attachment #266555 -
Attachment is obsolete: true
Attachment #266555 -
Flags: superreview?(roc)
Attachment #266555 -
Flags: review?(roc)
Assignee | ||
Comment 22•18 years ago
|
||
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•18 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•18 years ago
|
Whiteboard: [checkin needed]
Comment 25•18 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
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Comment 26•18 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•18 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.
Comment 29•18 years ago
|
||
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•18 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•18 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•18 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•18 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.
Assignee | ||
Comment 36•18 years ago
|
||
Vassil's followup hasn't been checked in yet, adding smoke signal to whiteboard.
Whiteboard: [checkin needed]
Comment 37•18 years ago
|
||
The border around a scrollbar in a list seems to light up on mouseover with this follow up patch.
Comment 38•18 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•18 years ago
|
||
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•18 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]
Comment 42•18 years ago
|
||
This doesn't seem to play very nice with canvas... See bug 384266.
Comment 43•18 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.
Comment 44•17 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•17 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•17 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•17 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•17 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•17 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.
See Also: → https://launchpad.net/bugs/40051
You need to log in
before you can comment on or make changes to this bug.
Description
•