Bug 46990 - LibreOffice fails to recognize MATE as GNOME/GTK-based desktop
Summary: LibreOffice fails to recognize MATE as GNOME/GTK-based desktop
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
3.4.5 release
Hardware: All Linux (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:4.2.0
Keywords:
: 47452 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-03-05 22:48 UTC by Arnaud Champollion
Modified: 2013-11-05 13:09 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
add MATE and XFCE to standard fallback list (3.32 KB, patch)
2013-07-01 15:43 UTC, rezso
Details
add MATE and XFCE to standard fallback list - try2 (3.33 KB, patch)
2013-07-02 16:47 UTC, rezso
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arnaud Champollion 2012-03-05 22:48:28 UTC
I have LibreOffice 3.4.5
OOO340m1 (Build:502)

on Debian Wheezy, with MATE desktop.


In LibreOffice, it's impossible tu use GTK open/save dialog boxes as you can in Gnome.


The problem have been "solved" by adding

export OOO_FORCE_DESKTOP=gnome

to ~/.profile

and

if [ -r ~/.profile ]; then
    . ~/.profile
fi

to ~/.xsessionrc


Here is the conclusion of Mirosław Zalewski, who helped me on the user mailing list :

LightDM on login reads script set in session-wrapper in
/etc/lightdm/lightdm.conf file. On Debian, this is /etc/X11/Xsession.

That script sets up X environment, by reading files in /etc/X11/Xsession.d
directory. One of them sources users' XSESSIONRC file, which is ~/.xsessionrc.

In my solution, I tell to put there statement, that reads ~/.profile if it
exists and is readable. Then I tell to put OOO_FORCE_DESKTOP in that ~/.profile
file.

So, in fact, you could put OOO_FORCE_DESKTOP in ~/.xsessionrc and it should
work. I suggest to put in ~/.profile file, because that way you can have all
environment variables in that one single file. If you ever happen to switch
from LightDM to other graphical login manager, or decide to not use any
graphical login manager at all, all your variables should be in place (or will
be with minimal configuration).

Here is an answer of Michael Meeks, on the developper list :

Ok - so this is used in this code:

http://cgit.freedesktop.org/libreoffice/core/tree/vcl/unx/generic/plugadapt/salplug.cxx#n158

	and the black-magic desktop detector is here:

http://cgit.freedesktop.org/libreoffice/core/tree/vcl/unx/generic/desktopdetect/desktopdetector.cxx#n50

(...)

If it is certain that your desktop always wants to use the gtk+
backend, -and- it is possible to detect your desktop reasonably quickly
& easily (ie. an environment variable would be best).

	Then we can trivially add this to the desktopdetector.cxx and fix all
lightdm users in our next release.
Comment 1 Urmas 2012-08-19 04:57:55 UTC
Why should one clog the codebase with support for toy, ephemeral projects like Mate?
Comment 2 Joel Madero 2013-06-10 16:07:38 UTC
@Aarnud - please ignore Urmas' comment - he does not speak for LibreOffice, nor for our QA team.

Has the problem been solved by 4.0 or should I try to get a VM going - apologies for the long delay, our QA team is stretched pretty thin but this one looks pretty bad. 

If you can just confirm it's still an issue I'll put in the time to install on a VM and confirm.

Again, thanks for your patience
Comment 3 rezso 2013-07-01 15:43:34 UTC
Created attachment 81821 [details]
add MATE and XFCE to standard fallback list

My solution for this: I added MATE and XFCE desktops to known desktops, and forcing use the standard (gnome) fallback for both.
In addition, I added an getenv( "DESKTOP_SESSION" ) for gnome check, because the GNOME_DESKTOP_SESSION_ID is obsolete.
Comment 4 Michael Meeks 2013-07-01 16:01:22 UTC
nice work :-) so - a few requests; could you change the patch to:

static bool is_xfce_desktop( Display* pDisplay )

to have a single return statement in the body ?

Also - can you test this with an 'unset DESKTOP_SESSION' ? - it's not obvious that the OUString constructor will cope with NULL nicely.

It's entirely possible that we want to extract and convert that DESKTOP_SESSION into an OString just once to avoid a number of getenv() calls which just waste time (and the malloc/free of constructing an OString).

Now I read that code more I'm a bit annoyed that we do all this X property / heavy lifting for TDE un-conditionally in there - IMHO it'd be good to disable that for performance if the backend is not compiled in :-) It'd also be nice to avoid doing the KDE / X property foo as well for performance. At least, until we have no env. vars to give us security.

This chunk:

+    else if ( desktop == DESKTOP_XFCE )
+        pList = pStandardFallbackList;
+    else if ( desktop == DESKTOP_MATE )
+        pList = pStandardFallbackList;

Would look nicer with a pair of || clauses in the if( GNOME ) above I think.

Any chance of some improvements like that ? :-)

And thanks for contributing !

BTW. it's preferable to send patches to the developer list with [PATCH] in the Subject I think, rather than attaching in bugzilla; please do CC me too ...
Comment 5 rezso 2013-07-02 16:47:45 UTC
Created attachment 81894 [details]
add MATE and XFCE to standard fallback list - try2

Thanks for your notes, Michael.

About your requests:

I reworked the is_xfce_desktop() and is_mate_desktop() with return, and added desktop == DESKTOP_XFCE and desktop == DESKTOP_MATE to desktop == DESKTOP_GNOME with an || operator.

unset DESKTOP_SESSION: why? If any user log in to any desktop environment, this environment variable automatically gets a value.

Unconditional TDE check:
- desktopdetector.cxx, static int TDEVersion( Display* pDisplay ): if I use a condition for TDE, why not apply a condition for KDE and KDE4? Same in DESKTOP_DETECTOR_PUBLIC DesktopType get_desktop_environment(), and const char* pPlugin[] = { "gtk3", "gtk", "kde4", "kde", "tde", "gen", 0 }; in salplug.cxx.
- salplug.cxx: static const char* pTDEFallbackList[] - can you imagine a TDE environment / fallback without TDE? :)
So I don't know...

I'm not an expert in LO code and programming, my patch just a quick hack, which works for me (I use ~2 months ago).
Comment 6 Michael Meeks 2013-07-02 21:05:25 UTC
> I reworked the is_xfce_desktop() and is_mate_desktop() with return

Ah - I was expecting a simple { return "mate" == getenv(); } method :-)

> unset DESKTOP_SESSION: why? If any user log in to any desktop environment,
> this environment variable automatically gets a value.

True today, but certainly not true for older machines, DESKTOP_SESSION is a new innovation it seems ;-) and crashing for a NULL env. var is really lame - so we should avoid that.

> I'm not an expert in LO code and programming, my patch just a quick hack, 
> which works for me (I use ~2 months ago).

I pushed another patch which made it somewhat simpler and hopefully easier to read. Anyhow - thanks for your work here ! good to get this fixed :-) any other bugs / easy hacks on your radar ?
Comment 7 Commit Notification 2013-07-02 21:10:04 UTC
Pader Rezso committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f31fad32005c6709eaae71b49af31264e585478a

fdo#46990 - detect MATE and XFCE desktops.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2013-07-02 21:10:22 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d2a71b952850cdf1a7efc5c0f86df9864201a950

fdo#46990 - re-work new desktop checks, guard against NULL DESKTOP_SESSION.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 9 rezso 2013-07-02 22:10:05 UTC
(In reply to comment #6)
 
> Ah - I was expecting a simple { return "mate" == getenv(); } method :-)

Okay :)

> True today, but certainly not true for older machines, DESKTOP_SESSION is a
> new innovation it seems ;-) and crashing for a NULL env. var is really lame
> - so we should avoid that.

I agree this.

> I pushed another patch which made it somewhat simpler and hopefully easier
> to read. Anyhow - thanks for your work here ! good to get this fixed :-) any
> other bugs / easy hacks on your radar ?

Thanks for your quick and professional work :)
Other bugs / quick hacks on my radar:
- sysui/desktop/mimetypes/openoffice.applications contains an 'uses_gnomevfs=true' line, but I compile LO without gnomevfs support. So I use a sed -i 's/uses_gnomevfs=true/uses_gnomevfs=false/g' sysui/desktop/mimetypes/openoffice.applications command in my build script.
- configure: change some defaults:
--disable-odk -> --enable-odk (I think, ODK is not useful for end-users :))
--enable-gtk3 -> --disable-gtk3 (current gnome depends on gtk3)
--disable-gconf -> --enable-gconf (gconf replaced with dconf and gsettings)
--disable-gio -> --enable-gio
--disable-tdeab -> --enable-tdeab (if the tde support needs an --enable-tde option, why --disable-tdeab the default?)
--disable-kdeab -> --enable-kdeab: same as tdeab
--without-java don't set --disable-system-jars
Comment 10 Maxim Monastirsky 2013-11-05 13:09:55 UTC
*** Bug 47452 has been marked as a duplicate of this bug. ***