Issue 61174

Created attachment 33569 [details]
patch for change User Interface colors on Mac OS X

fixed in aquacolors 

*** Issue 61173 has been marked as a duplicate of this issue. ***

*** Issue 61172 has been marked as a duplicate of this issue. ***

ericb: sorry but I do not like to change the colors this way.

Where are UI colors used? Can't we simply use the new colors triplet there
instead of COL_BLUE and COL_LIGHTGRAY and not change the real COL_*?

Right now *all* places where these colors are used are affected. This is wrong
approach. You should change *only* the place(s) for UI colors.

ericb->pjanik

In fact, I already was searching in that direction : in my post on mac@porting I
was talking about " #include <color.hxx> "used 445 times, but just some times
for COL_{BLUE | LIGHTGREY}. 

I'm investigating exactly were..

And I agree, directly use the triplet just when needed is the better choice, and
I'll change that way.  I wanted to say I still have not found a bad case.

The bad side is around 25 modules (maxi) are concerned, and this changes a lot
of things..

ericb->pjanik

Other idea : 

Maybe define new colors like COL_BLUE_AQUA and COL_LIGHTGRAY_AQUA is more clean,
to avoir #ifdef MACOSX ... #endif

BTW : I found a lot of COL_LIGHTGRAY and COL_BLUE 

Created attachment 33590 [details]
as reminder, the list of COL_BLUE I found

Created attachment 33591 [details]
the same for COL_LIGHTGRAY

Ah well, so I guess this won't be a quick fix after all.

I agree that it does not make sense to redefine color BLUE to something else 
for the whole application (that would make the text documents and 
presentations use different colors on Mac OS X). Only UI should be affected.

Interesting files from the gray-list:
tools/inc/tools_inc_colors.hxx_ui_color.diff
vcl/source/app/settings.cxx
tools/source/generic/color.cxx
sw/source/ui/config/viewopt.cxx
vcl/unx/gtk/gdi/salnativewidgets-gtk.cxx

Would one of these solutions be good? :
a)  in color.hxx, define new COL_UI_BLUE, COL_UI_LIGHTGRAY and map them to the 
existing colors by default (and to aqua colors on OSX). Then change all the 
places that affect the UI: from COL_BLUE to COL_UI_BLUE.

b) How gtk themes do this? Is there a function or a setting somewhere that 
would allow us to just specify a configure setting or to set the correct 
colors in runtime?

Sorry, but I must say that the current patch is the wrong approach and must not
be integrated! VCL provides functionality to adapt the system colors and you
have to set the correct colors into the corresponding settings and not redefine
what blue or gray should be.

The function void DtIntegrator::GetSystemLook( AllSettings& rSettings ) in
vcl/unx/source/gdi/dtint.cxx can be used to fill in the right colors. There are
multiple Integrators available but on plain X11 the base class (DtIntegrator) is
used, so you can put an #ifdef MAC block here and provide your colors in the
StyleSettings object. Have a look at the other GetSystemLook() implementations
to get an idea. You should start with setting the face color which is used quite
often, so you can easily see your changes and then continue from there.

Again, please forget the idea about redefining color values ! A patch like this
will never be integrated.

adding me to CC:.

ericb->ssa 

Please correct me if I'm wrong :

DtIntegrator::GetSystemLook uses StyleSettings::StyleSettings() ( found in setting.cxx)  
In setting.cxx, StyleSettings::StyleSettings() allocate memory for a pointer on ImplStyleData(), 
and  ImplStyleData() does contain a lot of definitions, included colors.

What Ì do not understand : can we modify directly the content of ImpleStyleData(), or do we create a 
new similar object (where ? ). For example replacing directly could give :

maFaceColor  = Color (COL_LIGHTGRAY);
+ all color changes to test 
with : 

replaced by :

#ifdef MACOSX
  maFaceColor  = Color (RGB_COLORDATA(0xF4, 0xF4, 0xF4));
  + all color changes to test on Mac OS X
(same for COL_BLUE)
#else
  maFaceColor  = Color (COL_LIGHTGRAY);
  + all collor changes for !MACOSX
#endif 
(same for COL_BLUE)

Or something completely different must be done ?


Thank's in advance for  any help/sugggestion

Created attachment 33627 [details]
First try at implementing GetStyleSettings for Mac OS X

Created attachment 33628 [details]
A settings resources file for color settings

Using kde files from OOo 2.0.1 as a template, I have made a first try at
implementing the color settings for Mac OS X.

Since the KDE version uses a file with the settings, I have used this approach
in the Mac OS X version as well. This enables the user to manually change the
colors to her liking.

The patch will not work out of the box and probably will not compile either.
Anyone willing to help to make the necessary modifications to makefiles and
delivery stuff? 

All the necessary .hxx and .cxx have been created. When it compiles and
delivers, it will be probably necessary to define environment variable
"OOO_FORCE_DESKTOP=macosx" to get it working as automatic detection will not
probably work.

The settings file should be put to "~/Library/Application Support/OpenOffice.org2/"

Created attachment 33629 [details]
A new version that may compile

The v2 of the patch may compile. But does it deliver? It's all so confusing...

Sidenote: the IZ 59303 does the _opposite_ for kde (cws vcl50)

Created attachment 33630 [details]
updated colors

I have successfully compiled a local build (OOo 2.0.1) with these changes and
the themed colors appear :) (requires the OOO_FORCE_DESKTOP=macosx -variable)

The colors are themed in almost all the other places, but the main menu is not
themed. (bug somewhere)

If you change the colors in the rc file, you have to restart OOo to see the changes.

ericb->mox

One more time you did a great work ! I really learned a lot of things.  Unfortunaly, I'm working all the day, 
so I won't be able to test before this evening (sure I'll do)

Waiting, Stephan will give his opinion/suggestions, and if things are ok, I'll commit your changes (if none 
disagree)

ericb->mox

Your patch does not apply on m154, because some kde stuff has been removed.

I have written a patch that you can apply with m154.  vcl builds fine with it on m153, but I don't know if 
I'm wrong or not.

The problem I have is :

when instantiate MACOSXIntegrator()  ?  In your code, it was made twice, and I have not understood 
why. From my side, instead of use (KDEIntegrator is missing in m154) :

""""""""""""""""""""""""""""""""""
 @@ -118,6 +121,8 @@
 #endif
         if( aOver.equalsIgnoreAsciiCase( "kde" ) )
             return new KDEIntegrator();
+        if( aOver.equalsIgnoreAsciiCase( "macosx" ))
 # nclude <sareturn new MACOSXIntegrator();
         if( aOver.equalsIgnoreAsciiCase( "none" ) )
   )         return new DtIntegrator();
     }
@@ -138,6 +143,9 @@
+    if( pSalDisplay->getWMAdaptor()->getWindowManagerName().EqualsAscii( "Syste
mUIServer.app" ) )
+               return new MACOSXIntegrator();
+
        // default: generic implementation
        return new DtIntegrator();
 }
""""""""""""""""""""""""""""""""""""""""""""""""""""""""""

I just create MACOSXIntegrator if WindowManagerName().EqualsAscii is SystemUIServer.app  :

"""""""""""""""""""""""""""""""""""""""""""""""""""""""""
@@ -142,8 +145,12 @@
        }
 #endif
  
+    if( pSalDisplay->getWMAdaptor()->getWindowManagerName().EqualsAscii( "SystemUIServer.app" ) )
+               return new MACOSXIntegrator();
+    else
"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""

I have not tested. Of course, if I'm wrong, please let me know

Created attachment 33665 [details]
patch adapted to m154 source code

ericb: the first instantiation is used when you force the integrator with
OOO_FORCE_DESKTOP=macosx environment variable. (this works)

The second instantiation is when autodetection is working. (but this does not
work currently). I don't know the proper name for WindowManagerName() on Mac OS
X. The current thing is just a placeholder.

ericb->mox

Ok, thank's for your quick answer :-)  Anyway, m154 is building, and I'll test/modify what needs to be 
tomorrow, or this night if the build ends not too late.

IMHO, with two instanciations, one is too much, and it should work with only one.

BTW Kendy may certainly help us too, and I'll ask him some tips tomorrow.

Created attachment 33668 [details]
Color settings for v4

Created attachment 33669 [details]
Color settings patch v4

I'm still building OOo 2.0.1, but I applied the kde-removal patch to it, so the
v4 of the patch should apply pretty well to m154 as well.

The latest patch sets more colors and thus also the menubar uses now correct colors.

Could someone please provide a correct line to this:
fprintf( stderr, "getWindowManagerName(): \"%s\" \n",
pSalDisplay->getWMAdaptor()->getWindowManagerName());

Currently that crashes, because I don't know how to handle the string
conversions/getSomethings

Stumbled upon a similar enhancement in NeoOffice at
http://bugzilla.neooffice.org/bug.php?op=show&bugid=692
They use a more brute force method, but more interesting is, should we use the
same colors as them? Does that need permission from Patrick?

mox: the correct line would be

fprintf( stderr, "getWindowManagerName(): \"%s\" \n",
rtl::OUStringToOString( pSalDisplay->getWMAdaptor()->getWindowManagerName(),
osl_getThreadTextEncoding() ).getStr() );

also please surround the creation of MACOSXIntegrator with #ifdef MACOSX like
you did with the #include directive, else the code will not compile on not
MacOSX. Also you might want to make the compilation of macosxint.cxx conditional
for MacOSX (like the compilation for CDEIntegrator is conditional for Solaris,
please have a look at makefile.mk

Aside from these minor technicalities the patch looks good to me, if you want to
go with an RC file. Are there possibly methods to ask the Mac API for these
colors instead of using a file ?

ericb->pl

IMHO, a config file is an intermediate, but sufficient solution : we'd prefer concentrate on native 
changes, and stop with X11 development. Mox and me are working on it because change the look was 
a big request : 1 Mac user over 2 complains about the awfull "grey and blue" used in the interface. 

This is of course a little change regarding the size of the existing source code, but very important, 
believe me.

In fact (please, Mox, correct me if I'm wrong), we don't know how and/or if it's possible to use existing 
code to change colors on the fly, so any suggestion is welcome : it's 10 times more hard to discover 
everything than have some tips to start, like : prefer use that, this method  does exist, this object will 
help you, an example is there, have a look at ... 

Last but not least, what I have learned around this change was very interesting, and I'm the first ready 
to learn more.

...and thank you very much for your time :-)


ericb->mox

About the color, the aqua one looks good for me. I'm not sure neither we will have to ask Patrick if we 
want to use the same, but sure, it's more fair/correct to do this way. 

About your patch : I have used your changes, but I cannot apply them directly to m154. So, after some 
modifications it works. Last, you'll have to modify the place where macosxrc is located. 

Correct path is : ~/Library/Application Support/OpenOffice.org 2.0

Other changes we have to do are : protect the build for other archs and commit the changes.
 
For that, discussing with obr 5 minutes ago on IRC, remaining modifications could be :

desktop  : 
- put macosxrc file in desktop/macosx/misc
- deliver it in solver

scp2 :

- add macosxrc file 
- maybe modify profileitem 

And if it still does not work, that's because I forgot something :-)

ericb: Changing colors on the fly in OOo or the desktop ? To make OOo read the
system settings again, simply call SendInternalEvent( pThis, NULL,
SALEVENT_SETTINGSCHANGED ) on your SalDisplay - pThis is a pointer to a live
SalFrame. this will cause SalFrame::UpdateSettings (and hence
DtIntegrator::GetSystemLook) to be called again.

ericb->pl

Thank you very much for the information. Very interesting, and I'll take note. To be more precise, I 
mean change the content of the macosxrc file from OOo, e.g. using a dialog box. IMHO, this is not 
necessary to implement such feature. Just one simple config file with Aqua colors,and  the objective is 
reached : stop to use the default grey/blue :-)


ericb->mox

I'll attach all the changes I have in mind (excepted scp2 not tested, the rest works) :

- in desktop  :  create macosx/source/misc directory, with macosxrc and corresponding makefile (to 
make macosxrc delivered) + prj/build.lst and prj/d.lst modified + soffice_lean.sh modified wit 
OOO_FORCE_DESKTOP environment checked, only if uname returns Darwin ( means Mac OS X is 
running)

- in vcl (aqua-colors-v7.patch) : my changes, in sync with m154, including #ifdef MACOSX , to protect 
other archs/OS for the build

- scp2 : add macosxrc in packaging, with good directory (I'm not sure, and I have not verified)

What is not clear, is in macosxint.cxx :  obr (Oliver Braun) told me about use Bootstrap::get ( in rtl/
bootstrap.hxx), using something like :

OUString Bootstrap::get( OUString(RTL_CONSTASCII_USTRINGPARAM("BaseInstallation")); 
BaseInstallation, OUString);

But I have not found (using bootstrap.hxx as example) how to use it correctly in macosxint.cxx.

The idea is to search first in ~/Library/Application Support/OpenOffice.org 2.0 if macosxrc does exist, 
and if not, search -using Bootstrap::get()-  in <INSTALL_DIR>/share/config, 
whith <INSTALL_DIR> ==  /Applications/OpenOffice.org 2.0.app/Contents/openoffice.org2, by default, 
but not always...

This way, we could have a default configuration, and for the users interested with experiences, they will 
just have to create macosxrc in ~/Library/Application Support/OpenOffice.org 2.0

Our problem is to check default macosxrc wherever OpenOffice.org 2.0 is installed ...
Of course, all attached files are draft, and I wait for opinion before to commit something.

Last but not least, I'll be away all the week, so I'll probably slow down the work for some times.

Created attachment 33725 [details]
status of aquacolors (scp2 not tested)

mox->pl: 
Thanks for the correct fprintf! It worked fine, although it turns out that on
MacOSX it returns just an empty string.

Yes, there are native functions to query the colors. However, this is not really
worth the effort, because:
a) X11 is not native looking, so there's no point in making lot of effort on
that side
b) Mac OS X actually uses pictures and patterns in some places in the UI, where
OOo only defines a single color. Thus the single colors that we define in
rc-file are an approximation, not a direct match to a system color.

...

mox->ericb:
Thanks for the scp2 and desktop work! It would have taken me years to understand
that stuff :)

I think it is more userfriendly to have the macosxrc only in user settings
directory. This way, all the user has to do, is to browse to that directory and
click on the file to edit. Actually, perhaps the filename should be
macosxrc.txt, so that it would open in TextEdit by default.

If we use a separate macosxrc in system settings directory, then user would have
to browse inside the .app, copy that file to correct place (user settings dir)
and then edit it. This would be much more difficult to do correctly.

...

Since the pSalDisplay->getWMAdaptor()->getWindowManagerName() returns an empty
string on Mac OS X, I think we should just brute force the "return new
MACOSXIntegrator();" on Mac OS X (i.e. no need for OOO_FORCE_DESKTOP). It can be
loaded always. 

I am not sure, but it seems that those who use GTK or KDE vclplugins in Mac OS
X, the vclplugin will override the settings in MACOSXIntegrator. So even when
vclplugins are used, it would be OK to brute force the "return new
MACOSXIntegrator();"

Oh BTW, 
the m154 still includes the files "vcl/unx/source/gdi/kdeint.cxx" and
"vcl/unx/inc/kdeint.hxx", even though all the other KDEIntegrator stuff was
removed... It is not built in the makefile.mk anymore. Maybe we can do a little
cleanup at the same time :)

...

The following v8 of the code uses the ideas that I presented before, and also
finishes the KDE cleanup.

That code is against m154 and should be ready to integrate to aquacolors cws and
for testing. Is there anything that should be done differently?

Created attachment 33753 [details]
aquacolors v8 patches

FYI: kdeint.[hc]xx removal is already done in CWS vcl54 (issue 61234) :-)

Created attachment 33927 [details]
Updated patch set. Does not affect kde-files. Ready for integration to the cws aquacolors

The v9 of the patch should applies against m154 and probably later ones as well.
It uses macosxrc.txt as a configuration file.

This should be now tested and integrated to cws.

ericb->mox
I have solved the scp2 problem, and I'll be able to commit tomorrow morning.
Today it was not possible to commit (see #i61941# ).

Before, last thing to do is to resync with m156 .

macosxrc (or macosxrc.txt if you prefer) will go -as expected- inside
~/Library/Application Support/OpenOffice.org 2.0/user directory.  (macosxint.cxx
has been modified to match this new dir )

Concerned modules for aquacolors are  :  vcl, desktop and scp2 if you want to
give it a try.

Important : we discussed a lot on IRC with Oliver Braun about the best places
for put everything. That's the reason why we decided to not do something too
complicated (use .xcu ) just for this little change.

Note : Oliver is my QA team for aquacolors cws.

mox->ericb:

Ok, the user -folder is good.

Yes, I want the file to be "macosxrc.txt"

I do not quite understand what is the problem with scp2, but it doesn't matter,
I'll check the cws, once you've committed...

Please note that my v9 patch above does not use OOO_FORCE_DESKTOP and that
environment variable should not be used for cws aquacolors.

re-target to 2.0.3.

CC for Oliver (my QA)

ericb->mox

We have tried using macosxrc : a double click on it and TextEdit can open it.
If it really is a problem, I'll change for macosxrc.txt.

Fixed in aquacolors, but waiting for Oliver opinion before to declare this cws
ready for QA

Yes, macosxrc can be opened with TextEdit, but manually. 

The idea is that macosxrc.txt is _by default_ opened with TextEdit, i.e.
automatically. This is a more userfriendly approach. 

Created attachment 34380 [details]
v10 of the patch

The v10 of the patch fixes the compilation of scp2 -module.

@all: Although I signed the JCA already in January, it still has not appeared to
http://www.openoffice.org/copyright/copyrightapproved.html :( Do you know who I
should ask if my JCA is still in some queue somewhere?

Created attachment 34411 [details]
updates to the UI font settings (see my comment in i57252, on Feb 23)

ericb->mox

Please check the last changes in aquacolors.

-> Following your recommandations, I have :

- renamed macosxrc into  macosxrc.txt
- modified default font for Lucida Grande (great choice!)
 
In runtime :  
when macosxrc.txt is not present in ~/Library/Application Support/openOffice.org 2.0/user folder, a 
default macosxrc.txt , present in <install_dir>/preset is used. bootstrap key mechanism has been used 
(thank's to JoergB  for his time)

Build (m156) currently in progress (vcl is successfull, binfilter 125th part over 148 is building) 

If everything goes fine for me, I'll set aquacolors as ready for QA

ericb->obr

I forgot to mention : aquacolors is only resynced with m156.

Your changes look good (checked from bonsai):
http://go-ooo.org/bonsai/cvsquery.cgi?branch=cws_src680_aquacolors&branchtype=match&date=month

Fixed in aquacolors

ericb->pjanik : may you please verify the changes does not break Linux nor Windows builds ? thank's in 
advance


Suggested QA process :

Point 1 :build should be ok for all archs/OS's : no interaction

Point 2 :Install process : 

1 .dmg installation 

Expected results : 
- an existing macosxrc.txt is really in <install_dir>/presets 

To verify : just right click -> show/display the contents, and see inside presets  to verify

Else : test fails

2) user install : Proceed a first Start of OpenOffice.org as simple user

Expected results : 
- a macosxrc.txt file must exist in ~/Library/Application Support/OpenOffice.org 2.0/user
- UI colors must be Aqua (pretty blue and very light grey)
- UI font must be Lucida Grande (Apple recommandation)

Else : test fails

Point 3 : User's config file must have priority

Context : user install is already done, without any modification

Modify the font in <install_dir>/presets : e.g. add a  "#" , means comment the line in 
<install_dir>/presets/macosxrc.txt and restart OpenOffice.org for the user :

Note : you probably need to authentify and change rights ( setting +w the file, default is -w )

Expected result :  No change for  the UI font : Lucida Grande font must continue be used
 
Else => test fails

Point 4 : Default file must replace missing user's file

1) As simple User rename macosxrc.txt in macosxrc.txt.old in 
 ~/Library/Application Support/OpenOffice.org 2.0/user and restart OpenOffice.org 

Expected result : OpenOffice.org must restart with a simple font, but not Lucida Grande

Else => test fails

2) As simple user, rename macosxrc.txt the file previously renamed, and restart OpenOffice.org

Expected result :  Lucida Grande must be used for UI

Else => test fails.


IMHO, if this already works ;-)
 

Verified in my local builds.

Works exactly as described by Eric.

Can we rename desktop/macosx/source/misc/macosxrc to macosxrc.txt so it is named
as the final final?

This line is also not documented in the macosxrc.txt file:

#cursorFlashTime=0

Also please adjust the comment:

// if macosxrc is not found in user install dir, fallback to default file,
located in <install_dir>/presets

It should say macosxrc.txt.

ericb->pjanik

- macosxrc.txt is now used everywhere
- cursorFlashTime option has been removed : more safe. this options is just there to modify cursor 
blinking => This changge is completely out of the scope of this cws

- change in macosxint.cxx comment done.

Fixed in aquacolors

Verified in aquacolors.

closing