Closed Bug 57799 Opened 24 years ago Closed 21 years ago

Mozilla needs to respond to system Appearance Theme changed events (fonts/colours)

Categories

(Core :: XUL, defect, P3)

PowerPC
macOS
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: lordpixel, Assigned: lordpixel)

References

Details

(Whiteboard: Review attachments 22846 24197 and 23681 please)

Attachments

(9 files)

When the system wide Appearence theme changes, Mac OS sends some Apple events to inform applications of this change. These are documented here: http://developer.apple.com/techpubs/macos8/HumanInterfaceToolbox/AppManager/ProgWithAppearanceMgr/Appearance.94.html#13915 Currently, my work on bug 1004 and bug 46174 is revealing that mozilla does not respond to these events, hence when the appearence theme is changed Mozilla does not recognise this change until a new window is opened. We need to respond to these events by redrawing all open windows and discarding any cached appearence information we might have. (I'm not sure we do cache anything at the moment, but I believe this will be necessary for performance reasons. More on this as I discover more)
Need to find an owner for this one. Any suggestions? This is not a Themes bug since it is not a specific issue with Modern or Classic. I don't think this falls under Skinability or Event Handling either. ccing for help assigning this bug...
Sounds like XPToolit to me, taking & futuring.
Assignee: hangas → trudelle
Component: Themes → XP Toolkit/Widgets
Target Milestone: --- → Future
Agreed. I'd forgotten that Modern uses the system font, though naturally classic (and potentially other themes) care far more about what the rest of the OS looks like. Resummarising
Summary: Mac Classic Skin needs to respond to system Appearance Theme changed events → Mozilla needs to respond to system Appearance Theme changed events (fonts/colours)
Blocks: 46174
Blocks: 49144
Status: NEW → ASSIGNED
Blocks: 61827
Its time this bug had a real owner, even if it stays future. who should get it?
Giving to lordpixel, who made good progress on this...
Assignee: trudelle → lordpixel
Status: ASSIGNED → NEW
FIXED. Code is a bit rough, but it works. Respond to event by calling ChromeRegistry.RefreshSkins() Will attach patches
Status: NEW → ASSIGNED
Attached file New class to do appearance events (deleted) —
Attached file Header for above class (deleted) —
Patch is pretty rough. Breaks some new ground for me so please scrutinise. I think smfr should review as most of the Apple Events code seems to be his.
Keywords: patch, review
Whiteboard: Review attachments 22846 22848 and 23429 please
* for all new files, please use consider using mpl boilerplate license unless you are under a contract that requires you to use another. http://www.mozilla.org/MPL/boilerplate-1.1/ * please try to limit lines to 80 columns, * avoid tabs * follow the emacs modeline [23429 says 2 spaces but it looks like you're using tabs] there's a general warning about using C++ exceptions due to risks of crossing xpcom boundaries into an object compiled by a different compiler that handles exceptions differently. -- It looks like you do risk that.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
timeless: The mac Apple Event code has been written to use exceptions (mostly because it was derived from another project). Because it's mac-only code, I think this is OK. For some reason, timeless marked this fixed. reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
sorry about the resolution.
Status: REOPENED → ASSIGNED
Attached file .h file with similar cleanups (deleted) —
(no, I ain't *just* spamming you, I updated the status whiteboard too :-) grrrr. I hate tabs as spaces. Its 2001 and we can't get editors to do something sensible. I blame vi users for 8 space abominations!
Whiteboard: Review attachments 22846 22848 and 23429 please → Review attachments 22846 23638 and 23639 please
Uh, no one uses vi these days. vim has :set expandtabs, there is no excuse. I see no tabs in the .cpp file, but the try body is overindented by four spaces. Please change the Emacs modeline comments to set tab-width: 4 and c-basic-offset: 4, to match the code. /be
Attached file Brendan's cleanups applied to .h (deleted) —
Heh. Ok done. Perhaps I shoul have put in another smiley after that vi comment. Actually I use vi. I have to deal with Solaris 7 systems. Anyway, does someone want to r= this?
Whiteboard: Review attachments 22846 23638 and 23639 please → Review attachments 22846 23680 and 23681 please
http://www.vim.org -- it's open source. r=brendan@mozilla.org, maybe sfraser or another Mac dude should sr=? /be
Why do you need the try/catch block in AEAppearanceSuiteHandler::HandleAppearanceSuiteEvent, when the calling routine will catch exceptions for you?
I think I copied that from ?your? code in the Spyglass suite handler. I'll look at it when I get back this weekend...
Whiteboard: Review attachments 22846 23680 and 23681 please → Review attachments 22846 24197 and 23681 please
Since lordpixel seems to have a patch ready to go for this shouldn't it have a target of Mozilla0.9 rather than future?
The component is XP Toolkit, not themes. Please change the QA contact. Thanks!
QA Contact: pmac → jrgm
nominating for dogfood (from sdagley's list of bugs that are good candidates for our next release)
Keywords: nsdogfood
Keywords: nsCatFood
Keywords: nsdogfood
Has there been any movement on this? Is it ready to check in?
Still waiting for r= from a Mac person.
Yeah i suck
i would suggest moving #include "nsIChromeRegistry.h" #include "nsCOMPtr.h" from the .h file into the .cpp file. There's nothing in the .h file that references them. It's best to reduce includes to a minimum, especially in headers.
This could also be useful for Windows. (bug 6061, bug 67625 and others) In an earlier attempt to fix this, an event NS_DISPLAYCHANGED was created (bug 6061). At the moment only fired on Windows and currently unused. http://lxr.mozilla.org/mozilla/source/widget/public/nsGUIEvent.h#371 Would it be possible for the Mac to fire NS_DISPLAYCHANGED (or create a new event type for this if you don't like that name based on the Windows event) and handle the Refresh code in the event handler? That way we can use it on all platforms. I tried this out on Win32 and it seems to work fine. (I simply added RefreshSkins() code to PresShell::HandleEvent, not sure what the best place for this would be)
Target Milestone: Future → mozilla0.9
Sure, doing this in a consistent manner would be really cool. One thing though, we're not really fixing the same bug... what you're responding to is bit depth changes etc, whereas I'm responding to theme changes, i.e. changes to the whole look and feel of the operating system. This means I'll probably have to fire the event on Mac OS in 2 circumstances, 1/ when the colour depth changes 2/ when the theme changes You might want to fire it when a windows user changes the theme (color scheme in the Display control panel) on Windows? And what of GTK? Is there code which shows how to fire this eventt? I know next to nothing about Moz events.
bug 67625 is about switching colors (which happens in Windows when switching themes), when I'm correct this patch could all 'os appearance changes' when triggered after the change. On Windows we get a few messages like syscolorschange, displaychanged, settingschange when this happens. I don't know much about NS events, here is what I scrambled together: On Windows the event is fired with the line: DispatchStandardEvent (NS_DISPLAYCHANGED) In the function that listens to Windows events it does this with a case: http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp#2767 case WM_DISPLAYCHANGE: DispatchStandardEvent(NS_DISPLAYCHANGED); break; DispatchStandardEvent is defined on Windows as (there is not a Mac definition for this afaict): PRBool nsWindow::DispatchStandardEvent(PRUint32 aMsg) { nsGUIEvent event; event.eventStructType = NS_GUI_EVENT; InitEvent(event, aMsg); PRBool result = DispatchWindowEvent(&event); NS_RELEASE(event.widget); return result; } DispatchWindowEvent is defined on Mac: http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsWindow.cpp#1930 that file has a DispatchMouseEvent on line #1949 that seems to handle some Mac events, looks like it could be used as a template for this.
OK, so if you can provide your patch at the end of bug 6061 in diff/patch format, I'll apply it then try to figure out how to fire the event on Mac OS. That should give us a good solution...
Moving to 0.9.1 as per Marek's request. Need to decide how to proceed with a coherent story for handling theme switch and colour depth switches cross platform.
Target Milestone: mozilla0.9 → mozilla0.9.1
what are the chances of this landing in the next week or so. if not good lets unset the target milestone until we get a good plan in place.
I have no plan to resolve the outstanding issues at present. Needs research.
Target Milestone: mozilla0.9.1 → ---
Perhaps superficially related to Bug 6061 and Bug 101843?
Mozilla CFM build is dead; should this bug go with it?
This bug is targeted at a Mac classic platform/OS, which is no longer supported by mozilla.org. Please re-target it to another platform/OS if this bug applies there as well or resolve this bug. I will resolve this bug as WONTFIX in four weeks if no action has been taken. To filter this and similar messages out, please filter for "mac_cla_reorg".
OS: Mac System 8.5 → MacOS X
Classic support long dead
Status: ASSIGNED → RESOLVED
Closed: 24 years ago21 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: