Bug 148071 - change about info Mac OS X to macOS
Summary: change about info Mac OS X to macOS
Status: RESOLVED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All macOS (All)
: low trivial
Assignee: Not Assigned
URL:
Whiteboard: target:24.2.0
Keywords:
Depends on:
Blocks: macOS-UI-polish About-Dialog
  Show dependency treegraph
 
Reported: 2022-03-18 13:33 UTC by steve
Modified: 2023-09-28 20:33 UTC (History)
8 users (show)

See Also:
Crash report or crash signature:


Attachments
Change about info Mac OS X to macOS (421 bytes, patch)
2022-12-23 02:51 UTC, Sierk Bornemann
Details

Note You need to log in before you can comment on or make changes to this bug.
Description steve 2022-03-18 13:33:51 UTC
Description:
The OS is called macOS, lets use the correct name in about dialog.

Steps to Reproduce:
About about dialog

Actual Results:
Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: 94016b435404f19d84ed636c5b3fd1ab0c02e020
CPU threads: 8; OS: Mac OS X 12.3; UI render: default; VCL: osx
Locale: de-DE (en_DE.UTF-8); UI: en-US
Calc: threaded

Expected Results:
Version: 7.4.0.0.alpha0+ / LibreOffice Community
Build ID: 94016b435404f19d84ed636c5b3fd1ab0c02e020
CPU threads: 8; OS: macOS 12.3; UI render: default; VCL: osx
Locale: de-DE (en_DE.UTF-8); UI: en-US
Calc: threaded


Reproducible: Always


User Profile Reset: No



Additional Info:
Really nice to have back correct version info for macOS 12 Monterey by the way.
Comment 1 Julien Nabet 2022-03-18 17:36:35 UTC
I submitted this patch for review for master branch:
https://gerrit.libreoffice.org/c/core/+/131836
Comment 2 Julien Nabet 2022-03-24 07:17:30 UTC
Since it seems it can bring some pb according to the comment, I abandoned the patch.
Can't help here=>unassign myself.
Comment 3 Sierk Bornemann 2022-12-22 17:45:54 UTC
LO's About window unfortunately still says "Mac OS X" rather than "macOS".

Version: 7.4.4.1 / LibreOffice Community
Build ID: 988f4a351a6fa8cf4bdf2bdc873ca12cf8cbe625
CPU threads: 10; OS: Mac OS X 13.1; UI render: default; VCL: osx
Locale: de-DE (de_DE.UTF-8); UI: de-DE
Calc: threaded

Version: 7.6.0.0.alpha0+ (AARCH64) / LibreOffice Community
Build ID: 8635c9aa8c6f1078a9e220076d5a08daf30077e8
CPU threads: 10; OS: Mac OS X 13.1; UI render: Skia/Metal; VCL: osx
Locale: de-DE (de_DE.UTF-8); UI: de-DE
Calc: threaded

My first guess has been, it comes from reading the ProductName key value of /System/Library/CoreServices/SystemVersionCompat.plist (<key>ProductName</key><string>Mac OS X</string>) rather than that in /System/Library/CoreServices/SystemVersion.plist (<key>ProductName</key><string>macOS</string>)?

Short explanation concerning these two files and which one to choose or to prefer, see https://eclecticlight.co/2020/08/13/macos-version-numbering-isnt-so-simple/, section "Interpreted and JIT languages" and especially the last sentence of the last paragraph of that section.


Correction: as I see in the patch by Julian Nabet, it seems to be a tiny hardcoded string in /vcl/osx/salinst.cxx, line 890: https://gerrit.libreoffice.org/c/core/+/131836/1/vcl/osx/salinst.cxx#b890

Following that all – maybe it would be better anyway, to determine and extracting that info via the OS itself based on

/usr/sbin/system_profiler SPSoftwareDataType
resp.
/usr/sbin/system_profiler SPSoftwareDataType -detailLevel mini ?

or simpler getting it with

/usr/bin/sw_vers -productName
/usr/bin/sw_vers -productVersion

instead of hardcoding it per string?

BTW: /usr/bin/sw_vers seems to read /System/Library/CoreServices/SystemVersion.plist, if I interpret its manpage correctly.



@Rob, @Mike Kaganski, @Patrick Luby: maybe one of you to make a short and easy patch & commit or at least commit Julien's patch proposal?
Comment 4 Sierk Bornemann 2022-12-23 02:50:14 UTC
What about this tiny patch proposal (based on Julien Nabet's patch proposal above) and taking <key>ProductName</key><string>macOS</string> of /System/Library/CoreServices/SystemVersion.plist instead of hardcoding the string?

--- salinst.cxx     2022-12-23 03:24:22
+++ salinst.cxx.new 2022-12-23 03:26:33
@@ -952,7 +952,7 @@
     if ( sysVersionDict )
         versionString = [ sysVersionDict valueForKey: @"ProductVersion" ];
 
-    OUString aVersion = "Mac OS X ";
+    OUString aVersion = [ sysVersionDict valueForKey: @"ProductName" ];
     if ( versionString )
         aVersion += OUString::fromUtf8( [ versionString UTF8String ] );
     else
Comment 5 Sierk Bornemann 2022-12-23 02:51:23 UTC
Created attachment 184324 [details]
Change about info Mac OS X to macOS
Comment 6 Stephan Bergmann 2023-02-01 09:34:39 UTC
(In reply to Sierk Bornemann from comment #5)
> Created attachment 184324 [details]
> Change about info Mac OS X to macOS

Assuming this patch is about vcl/osx/salinst.cxx, I guess this change would still suffer from the open issues discussed at <https://gerrit.libreoffice.org/c/core/+/131836> "tdf#148071: change about info Mac OS X to macOS"?
Comment 7 Sierk Bornemann 2023-02-01 12:55:40 UTC
(In reply to Stephan Bergmann from comment #6)
> (In reply to Sierk Bornemann from comment #5)
> > Created attachment 184324 [details]
> > Change about info Mac OS X to macOS
> 
> Assuming this patch is about vcl/osx/salinst.cxx, I guess this change would
> still suffer from the open issues discussed at
> <https://gerrit.libreoffice.org/c/core/+/131836> "tdf#148071: change about
> info Mac OS X to macOS"?

I've read the comments carefully. And have tried to unterstand the code behind the functions in question. Answering your question, DOES it then really suffer from the open issues discussed in https://gerrit.libreoffice.org/c/core/+/131836 ? I mean and conclude: No. I think, it's unrelated, the feared *potentially* guessed side effects, untested because of lacking environment, would not come to action.
As I read and understand the comments in question on gerrit: nobody has substantiated their reservations with corresponding code passages, because in each case apparently the right environment was missing to get it out, e.g. "Unfortunately, not sure how to get the right string out of Mac M1 architecture as I don't own one.". BTW: I have posted several ways above, how to get the right string.

DO these speculated und guessed and NOT further investigated discussed potentially problems relate to THIS particulary issue in question? I guess, they have NOTHING in common with each other and DO NOT relate to each other, if I interprete the code correct – I think, the discussed *potentially* side effects are just what they are: pure fearing/guessing and without substance.

Take

https://help.libreoffice.org/latest/en-US/text/scalc/01/04060104.html#hd_id5787224

Where and how are these variables for Info("system") are hardwired? They very all look hardwired. There seems to be a MACOSX string anywhere, being a variable. Capitalized. Without space within. As in "#elif defined MACOSX".
But this string, we talk about here is "Mac OS X" (and should be changed to "macOS").
Two different purposes, two different aims, two different locations in the code with nothing in common with each other.

Further:
I have no Microsoft Office installed on my Mac, so I can't answer the open question, what Microsoft Office throws as System Info string, so I don't know, 
if PushString("Windows (32-bit) NT 5.01") ist still appropriate in https://git.libreoffice.org/core/+/refs/heads/master/sc/source/core/tool/interpr5.cxx#3263 or should be another string for compatibility reasons, I guess, it should be another string, but I can't have a look, because I have no MS Office. But that is another construction site, unrelated to THIS bug/issue in question here.

All reservations about changing the string "Mac OS X" to "macOS" in the about dialog, changing it dynamically via a system function, I see as speculation that cannot be substantiated on closer inspection. So, in my view, the string can be changed (and should use OS-supplied system functionality instead of hard-wiring it manually) without fear of negative side effects. Just my 2 cents. I'd be happy to be taught better.
Comment 8 Stephan Bergmann 2023-02-01 13:52:57 UTC
Assuming Julien's analysis at <https://gerrit.libreoffice.org/c/core/+/131836/1#message-db00d721aa39cf3dc96ed56e2afa71504cd068ef> of all the places where the AquaSalInstance::getOSVersion() value can end up is still accurate, then the remaining open question would be as per <https://gerrit.libreoffice.org/c/core/+/131836/1#message-8f5a9cca1bc35af53067fba7fab8d29b794bdd67>:

>> Step 4, branch 2:
>> extensions/source/update/feed/updatefeed.cxx:329:OUString UpdateInformationProvider::getUserAgent(bool bExtended)
>>
>> used by uno::Sequence< beans::StringPair > SAL_CALL UpdateInformationProvider::getUserRequestHeaders( const OUString &aURL, ucb::WebDAVHTTPMethod )
>> so in a public interface offapi/com/sun/star/ucb/XWebDAVCommandEnvironment.idl:48:    sequence<com::sun::star::beans::StringPair> getUserRequestHeaders
>>
>> It could be relevant to provide correct info in a public interface.
>
> maybe Cloph has an idea about this part
Comment 9 Sierk Bornemann 2023-09-28 14:00:58 UTC
Fixed Mon Sep 18 22:40:18 2023 with commit 957d4254d6ab13bfa5df01b728d3396f11c82e9a by Andras Timar <andras.timar@collabora.com>

The name is macOS since 2016
https://git.libreoffice.org/core/+/957d4254d6ab13bfa5df01b728d3396f11c82e9a%5E%21/vcl/osx/salinst.cxx

The name is macOS since 2016
https://gerrit.libreoffice.org/c/core/+/157033


Finally! Thanks, Andras!

Version: 24.2.0.0.alpha0+ (AARCH64) / LibreOffice Community
Build ID: f8e591ab9182e0a61c4ae5b8f77b166fcaeaa877
CPU threads: 10; OS: macOS 14.0; UI render: Skia/Metal; VCL: osx
Locale: de-DE (de_DE.UTF-8); UI: de-DE
Calc: threaded
Comment 10 Stéphane Guillou (stragu) 2023-09-28 14:09:44 UTC
Let's mark as properly fixed then, with a target whiteboard.
Thank you Andras and everyone else!
Comment 11 Andras Timar 2023-09-28 15:17:59 UTC
(In reply to Sierk Bornemann from comment #9)
> 
> Finally! Thanks, Andras!
> 
Oh, thanks. I did not know about this bug. I also thought about reading the OS name from /System/Library/CoreServices/SystemVersion.plist. But I did not bother in the end, because with more complicated code we would have got the same result: "macOS". Current LibreOffice does not run on older OSes where the name was different.
Comment 12 Sierk Bornemann 2023-09-28 15:57:33 UTC
(In reply to Andras Timar from comment #11)
> (In reply to Sierk Bornemann from comment #9)
> > 
> > Finally! Thanks, Andras!
> > 
> Oh, thanks. I did not know about this bug. I also thought about reading the
> OS name from /System/Library/CoreServices/SystemVersion.plist. But I did not
> bother in the end, because with more complicated code we would have got the
> same result: "macOS". Current LibreOffice does not run on older OSes where
> the name was different.

OK. Your choice. I would've made it more generic and asking/reading it from the system (via the plist's ProductName key or via /usr/bin/sw_vers -ProductName), instead of hardcoding it – just for the case, Apple changes the name again. But it' OK for me.

What about also providing the patch for the current LO 7.5 and LO 7.6 line?


And, while we are at it: what about also changing the OS version checking, reflecting Apples recent changes since macOS 13.3.1 providing Rapid Security Response (RSR) updates with its new additional version string? If the OS has got a RSR update and so a new version, LibreOffice currently can't reflect that, only shows the version provided by the ProductVersion string, not the new ProductVersionExtra string reflecting the Version including the RSR update in brackets:

13.3.1 vs. 13.3.1 (a)
13.4.1 vs. 13.4.1 (a) / 13.4.1 (c)

For that, this LibreOffice code fragment in vcl/osx/salinst.cxx has to be changed:
  
 versionString = [ sysVersionDict valueForKey: @"ProductVersion" ];
 aVersion = [ sysVersionDict valueForKey: @"ProductName" ];

not only reading the key ProductVersion from /System/Library/CoreServices/SystemVersion.plist
but also, if present, reading the key ProductVersionExtra from 
 /System/Cryptexes/OS/System/Library/CoreServices/SystemVersion.plist
(it seems to be there only, if present, and _not_ in /System/Library/CoreServices/SystemVersion.plist)

Or, just use

/usr/bin/sw_vers -ProductName
macOS

/usr/bin/sw_vers -ProductVersion
14.0

and if present:
/usr/bin/sw_vers -ProductVersionExtra
(c)

I assume, this is out of scope of this bug here, and a new bug should be filed and a further patch be committed for that. Could/would you please take care of it or initiate it? Thanks in advance.
Comment 13 Andras Timar 2023-09-28 20:33:22 UTC
(In reply to Sierk Bornemann from comment #12)
> Could/would you please take
> care of it or initiate it? Thanks in advance.

No, thanks. :) I'm even not sure now that my patch was right in the first place, see https://gerrit.libreoffice.org/c/core/+/157033 and discussions in this bug.