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)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: lordpixel, Assigned: lordpixel)
References
Details
(Whiteboard: Review attachments 22846 24197 and 23681 please)
Attachments
(9 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details |
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...
Comment 2•24 years ago
|
||
Sounds like XPToolit to me, taking & futuring.
Assignee: hangas → trudelle
Component: Themes → XP Toolkit/Widgets
Target Milestone: --- → Future
Assignee | ||
Comment 3•24 years ago
|
||
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)
Updated•24 years ago
|
Status: NEW → ASSIGNED
Comment 4•24 years ago
|
||
Its time this bug had a real owner, even if it stays future. who should get it?
Comment 5•24 years ago
|
||
Giving to lordpixel, who made good progress on this...
Assignee: trudelle → lordpixel
Status: ASSIGNED → NEW
Assignee | ||
Comment 6•24 years ago
|
||
FIXED. Code is a bit rough, but it works. Respond to event by calling
ChromeRegistry.RefreshSkins()
Will attach patches
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•24 years ago
|
||
Assignee | ||
Comment 8•24 years ago
|
||
Assignee | ||
Comment 9•24 years ago
|
||
Assignee | ||
Comment 10•24 years ago
|
||
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.
Assignee | ||
Comment 11•24 years ago
|
||
Assignee | ||
Updated•24 years ago
|
Whiteboard: Review attachments 22846 22848 and 23429 please
Comment 12•24 years ago
|
||
* 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
Comment 13•24 years ago
|
||
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 → ---
Assignee | ||
Comment 15•24 years ago
|
||
Assignee | ||
Comment 16•24 years ago
|
||
Assignee | ||
Comment 17•24 years ago
|
||
(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
Comment 18•24 years ago
|
||
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
Assignee | ||
Comment 19•24 years ago
|
||
Assignee | ||
Comment 20•24 years ago
|
||
Assignee | ||
Comment 21•24 years ago
|
||
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
Comment 22•24 years ago
|
||
http://www.vim.org -- it's open source.
r=brendan@mozilla.org, maybe sfraser or another Mac dude should sr=?
/be
Comment 23•24 years ago
|
||
Why do you need the try/catch block in
AEAppearanceSuiteHandler::HandleAppearanceSuiteEvent, when the calling routine
will catch exceptions for you?
Assignee | ||
Comment 24•24 years ago
|
||
I think I copied that from ?your? code in the Spyglass suite handler.
I'll look at it when I get back this weekend...
Assignee | ||
Comment 25•24 years ago
|
||
Assignee | ||
Updated•24 years ago
|
Whiteboard: Review attachments 22846 23680 and 23681 please → Review attachments 22846 24197 and 23681 please
Comment 26•24 years ago
|
||
Since lordpixel seems to have a patch ready to go for this shouldn't it have a
target of Mozilla0.9 rather than future?
Comment 27•24 years ago
|
||
The component is XP Toolkit, not themes. Please change the QA contact. Thanks!
Comment 28•24 years ago
|
||
nominating for dogfood (from sdagley's list of bugs that are good candidates for
our next release)
Keywords: nsdogfood
Comment 29•24 years ago
|
||
Has there been any movement on this? Is it ready to check in?
Assignee | ||
Comment 30•24 years ago
|
||
Still waiting for r= from a Mac person.
Comment 31•24 years ago
|
||
Yeah i suck
Comment 32•24 years ago
|
||
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.
Comment 33•24 years ago
|
||
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)
Updated•24 years ago
|
Target Milestone: Future → mozilla0.9
Assignee | ||
Comment 34•24 years ago
|
||
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.
Comment 35•24 years ago
|
||
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.
Assignee | ||
Comment 36•24 years ago
|
||
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...
Assignee | ||
Comment 37•24 years ago
|
||
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
Comment 38•24 years ago
|
||
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.
Assignee | ||
Comment 39•24 years ago
|
||
I have no plan to resolve the outstanding issues at present. Needs research.
Target Milestone: mozilla0.9.1 → ---
Comment 40•23 years ago
|
||
Perhaps superficially related to Bug 6061 and Bug 101843?
Comment 41•22 years ago
|
||
Mozilla CFM build is dead; should this bug go with it?
Comment 42•21 years ago
|
||
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".
Updated•21 years ago
|
OS: Mac System 8.5 → MacOS X
Assignee | ||
Comment 43•21 years ago
|
||
Classic support long dead
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 21 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•