[Needs Testing] Patch for Bug# 243078-Native Theme Rendering

Discussion of bugs in Mozilla Firefox
Post Reply
supernova_00
Posts: 4832
Joined: June 24th, 2004, 8:03 pm
Location: Maryland, USA

[Needs Testing] Patch for Bug# 243078-Native Theme Rendering

Post by supernova_00 »

Bug# 243078 [Core] - Native Theme Rendering for WinXP Menus, toolbars [Win]

James Ross has been doing a great job at getting Native Theme rendering on Windows to work correctly. James took on the bug after the recent checkin for Bug# 303806 (ugly menus for classic). Bug# 243078 is short on time for landing on branch for the Firefox 1.5 release and we can not let this patch miss the release and have firefox released half-assed looking so please, please test this patch throughly and report any bugs in this thread for James to see (please don't spam the bug). Thanks!

http://silver.warwickcompsoc.co.uk/temp/Firefox_1.5_Theme_Patch.zip (unzip into its own folder)

That's current trunk CVS from about 2 hours ago (2005091403 give or take a few hours), built with MSVC Toolkit with ActiveX disabled.

Known issues with the menus with patch:
  • The height of menubar items is still that of the containing toolbar.
  • Text colour for menubar items is incorrect when highlighted, when using certain flat-menu themes (e.g. standard Windows XP theme).
  • In Bookmarks menu, the items without images have their text at a slightly different horizontal alignment to the bookmarks (which have images). The "Open in tabs" item is the worst out-of-line.
Last edited by supernova_00 on September 17th, 2005, 3:25 pm, edited 3 times in total.
mw22
Posts: 2379
Joined: November 19th, 2002, 5:37 pm

Post by mw22 »

Well, the issues are quite clear, see:
https://bugzilla.mozilla.org/show_bug.cgi?id=243078#c37
Those need to be fixed, then I think the patch is ready to roll.
supernova_00
Posts: 4832
Joined: June 24th, 2004, 8:03 pm
Location: Maryland, USA

Post by supernova_00 »

Well ya but still needs wide testing before it would ever land. Needs testing with os themes, firefox themes, RTL text and other stuff. Plus like mconnor said, its a 300+ line patch so better we start testing it now so we can find bugs early enough for james to submit a new patch to be reviewd before it takes someone 3 weeks to review the patch and then find errors and takes another two weeks to make a new patch and get a review.
twpol
Posts: 70
Joined: August 8th, 2003, 1:24 pm
Location: UK

Post by twpol »

I'm just setting off builds on my home box, so I should be able to make a zip of my Firefox build available later today.

As for the "issues", the height thing is not something I will hold the patch up on - it is a minor detail compared to the rest. The biggest challenge is the text colour on the menubar.
mw22
Posts: 2379
Joined: November 19th, 2002, 5:37 pm

Post by mw22 »

I'll see what I can do. I can only test at earliest tomorrow.
I only build debug builds, so I can't really give a build with the patch included. If someone wants to try the patch, I would advise him to build himself. You're not dependable on others that way, and you can tinker with the code to see what happens.
supernova_00
Posts: 4832
Joined: June 24th, 2004, 8:03 pm
Location: Maryland, USA

Post by supernova_00 »

twpol: if you could email it to me to distribute, that would be great.
mw: i hear ya, i wish i could get a build to actually compile for once. plus my computer i'm on now would take about 30 hours to compile a build ;)
twpol
Posts: 70
Joined: August 8th, 2003, 1:24 pm
Location: UK

Post by twpol »

supernova_00, why do I need to? I'm just going to post a link to my University-hosted website, which can handle you guys downloading it. ;)
supernova_00
Posts: 4832
Joined: June 24th, 2004, 8:03 pm
Location: Maryland, USA

Post by supernova_00 »

alright, great then!
mw22
Posts: 2379
Joined: November 19th, 2002, 5:37 pm

Post by mw22 »

supernova_00 wrote:mw: i hear ya, i wish i could get a build to actually compile for once. plus my computer i'm on now would take about 30 hours to compile a build ;)

I'm sure people would like to help you. I sure am (I'm building with mingw).
I know it can be difficult/frustrating in the beginning.

edit:
Great patch, by the way, twpol/James! (except for the part that still sucks ;) )
twpol
Posts: 70
Joined: August 8th, 2003, 1:24 pm
Location: UK

Post by twpol »

http://silver.warwickcompsoc.co.uk/temp ... _Patch.zip (unzip into its own folder)

That's current trunk CVS from about 2 hours ago, built with MSVC Toolkit with ActiveX disabled.

Known issues with the menus with patch:
- The height of menubar items is still that of the containing toolbar.
- Text colour for menubar items is incorrect when highlighted, when using certain flat-menu themes (e.g. standard Windows XP theme).
- In Bookmarks menu, the items without images have their text at a slightly different horizontal alignment to the bookmarks (which have images). The "Open in tabs" item is the worst out-of-line.

I can't think of anything else I've found that isn't right, so please comment here if you spot something!
User avatar
BenBasson
Moderator
Posts: 13671
Joined: February 13th, 2004, 5:49 am
Location: London, UK
Contact:

Post by BenBasson »

Anyone know how to get this patch to apply on the branch? Trunk builds are nice for general feedback, but we need hard evidence of this working on the branch if it's going to land for 1.5.
twpol
Posts: 70
Joined: August 8th, 2003, 1:24 pm
Location: UK

Post by twpol »

If you don't know how to hand-apply a patch where it fails to apply itself, you are not in a position to be building it, IMHO.

Test the trunk build, and I'll get a branch build out by the end of the week (unless someone beats me).
User avatar
BenBasson
Moderator
Posts: 13671
Joined: February 13th, 2004, 5:49 am
Location: London, UK
Contact:

Post by BenBasson »

Thanks for that. I could easily hand-apply it, but if it's failing (since the files are in a different place and chunks of code are missing) it seems to me that there must be prerequisite patches and it's probably no use building without them.
twpol
Posts: 70
Joined: August 8th, 2003, 1:24 pm
Location: UK

Post by twpol »

There are no prerequisite patches, nor have files moved that I am aware of. The toolkit theme files will have been changed (probably slightly different from trunk because various people suck), but it is not hard to sort out. Anyway, as I said, I will do the work for you when I get back to my computer on Friday.
User avatar
BenBasson
Moderator
Posts: 13671
Joined: February 13th, 2004, 5:49 am
Location: London, UK
Contact:

Post by BenBasson »

Well, there's a distinct lack of mozilla/widget/src/windows/nsNativeThemeWin.cpp and mozilla/widget/src/windows/nsNativeThemeWin.h in my branch tree, which LXR confirms ;)
Post Reply