Description: With the current version of the iOS app (4.1.69) and iOS 13.2 it's not yet possible to use the multitasking feature as described here: https://support.apple.com/en-us/HT207582 Steps to Reproduce: 1. Try to open two (or more) instances of the iOS app Actual Results: It's currently not supported. Expected Results: It should be possible to open two documents at the same time. Reproducible: Always User Profile Reset: No Additional Info:
This bug should really be split into two: One about supporting SplitView (i.e. enabling the app to accept being used on just part of the display, instead of always being full-screen), and another about supporting intra-app SplitView, i.e. having multiple documents open in the same app instance. I assume it is the latter that is more important? (But it is interesting that not even the former feature seems to work, for some odd reason.)
Created attachment 155523 [details] Screenshot with Mail and the Collabora Office apps in SplitView on iPad No, wait, the app *does* already support the first kind of SplitView, sharing the screen with another app. I just wasn't used enough to the SplitView gestures, I kept swiping from the bottom too eagerly. One has to swipe quite gently.
Note to self: When/if starting work on this, this video is probably useful: https://developer.apple.com/videos/play/wwdc2019/258/
Also https://developer.apple.com/videos/play/wwdc2019/212
Created attachment 159583 [details] Screenshot of Split View Currently, when you split the screen between Collabora Office and another app (Twitter in the screenshot here), the menubar becomes vertical, and hides the leftmost part of the toolbar, including the "<" button, thus making it impossible to close the document. As a start, I will try to make the menubar always horizontal, and see whether it then becomes scrollable if it doesn't fit. (The "x" button in the top right corner appears, but that doesn't do anything sane, it just closes the document but keeps the top and bottom bars, does not return to the document browser part of the app.)
Created attachment 159612 [details] Screenshot On the other hand, if I make the menu bar horizontal also in narrow windows, it won't fit in a 50/50 split view on landscape orientation iPad. For English, the "Help" menu overflows and is put on a second row, which covers part of the toolbar. Looks messy and confusing, not good. (In other languages with longer words, most likely also the "Tools" menu will overflow.) And if you use 75/25 split view it is of course even worse, but in such a narrow Window Collabora Office is fairly unusable, probably nobody would do that anyway. So maybe it is better to let the menu become vertical for now. The "X" button that appears works now, so it doesn't just that much that the vertical menu covers the "<" button.
s/just/hurt
Note to self in case I jump on to something else and forget all details about this: The menu bar becomes vertical thanks to the media query in loleaflet/node_modules/smartmenus/dist/css/sm-simple/sm-simple.css: > @media (min-width: 768px) { > /* Switch to desktop layout that surrounds the part that makes the menu bar horizontal. For a split view window, the min-width becomes less than 768 and thus the the "Switch to desktop layout" isn't active.
@Tor: besides the menus there is an other aspect which I think is more complex to solve (not sure) and which is the root of the request from the users: They want to be able to have two documents (so two instances of the app) open and copy content from one document to the other. In my case the teachers have the big pads (12.9" IIRC) - so the size of the window is probably less of an issue. What is needed in order to make it possible to use two instances of the app at the same time?
Sure, sure, but before even attempting to make it possible to have the app "split view with itself" (which I think is what having two documents open at the same time in practice means), we need to make it somewhat usable even in split view with other apps, right?
I agree with you - performing well in split view with other apps would of course be a very nice milestone! I was just not sure, if I used the right wording to describe what the actual goal is. I imagine, that running the app twice, could be a nightmare regarding locking of resources, handling out of memory situations (#128048 comes to my mind), etc. I look very much forward to your work in this area, thanks a lot!
No matter whether we try to support having several documents open at the same time in some home-grown fashion that ignores the correct APIs that Apple provides for that purpose (as described in the WWDC videos above), or by using those APIs, the same issue needs to be solved anyway: That currently the iOS-specific code assumes there is just one document open at a time, and it uses some global variables related to that document, etc. That will have to be cleaned up in any case.
And not only are there iOS-specific global variables that need to be done away with in ios/ios.h: > extern int loolwsd_server_socket_fd; > extern lok::Document *lok_document; > extern LibreOfficeKit *lo_kit; but there are also global variables in the Online code, that in the web-based case are not problematic as they are per-process, but in the mobile app case the code that in web-based Online runs in a multitude of processes runs in a single process. Especially the TerminationFlag and MobileTerminationFlag comes to mind first. Getting the plumbing of the multi-process web-based Online to work in a single-process has been quite tricky and fragile, partly exactly because of these global variables.
After some discussion in gerrit it became obvious that the TerminationFlag is not that relevant after all; the current use of it in the mobile apps is based on a misunderstanding. In web-based Online it is apparently in the normal case set only if one interrupts the server for it to shut down. It should not be used in the mobile apps at all, so as a first step towards fixing this bug that misguided use of it will have to be removed.
More notes to self: More global variables that will have to be removed: In Kit.cpp: > static std::shared_ptr<Document> document; (Because there can be several documents open.) In the LOOLWSD class in LOOLWSD.hpp: > static std::mutex lokit_main_mutex; (Because there can be several lokit_main() processes, one per open document.)
And more: > static std::shared_ptr<lok::Document> _loKitDocument; in the Document class in Kit.cpp. And the related getLOKDocument() and getLOKitDocument() functions.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/online/commit/1525f774d9317e0f40d254b1f2af98344ba79daa tdf#128502: Get rid of the static file-level variable 'document' in Kit.cpp
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/online/commit/b94fb53d4d52bf3f847025aebbebe0383c41ffc8 tdf#128502: Make _loKitDocument in class Document non-static again
Another obvious change needed is that we shouldn't any longer let the LOOLWSD object's run() function terminate as in the loop in the thread dispatched in AppDelegate.mm. We should have one same LOOLWSD object all the time and its run() function should never finish.
Some more loosely written notes: After now having done modifications to get rid of the lok_document global, I notice the next hard issue: The Online code runs the lokit_main() function that runs the LibreOffice "main loop" per document. To be able to handle multiple documents, the "main loop" should run without being coupled to any specific open document. The KitWebSocketHandler is also coupled to just one document. When its handleMessage() function gets a "session" message it creates the Document object for the KitWebSocketHandler. More re-plumbing needed.
Out of curiosity; iOS does not spawn a second process for a second instance?
@Nicolas: Nope. See https://developer.apple.com/videos/play/wwdc2019/258/# "Why? Because applications now still just share one process but may have multiple user interface instances or scene sessions."
Created attachment 159994 [details] Screenshot. Getting closer. But I am sure there are lots of problems still. One obvious problem is that if you open the same document in multiple sessions, things very likely go very wrong. I wonder if it would be enough to just throw up an error message for such a situation? Or is there any good reason to allow it? (Yeah, "canonical" multiple-session-supporting apps like the bundled Notes app does allow you to have the same document open in multiple sessions, and if both are visible in Split View, then changes to one do show up immediately in the other. This is how this multiple-session thing is supposed to work, ideally. But in the Collabora Office iOS app case there are of course lots of added complications that probably makes it hard to support.
Created attachment 159996 [details] WIP patch
Awesome work Tor! Thanks a lot! I don't think making it possible to have the same document open twice is important. Showing a proper message is IMHO enough, my proposal would go in the following direction: "The chosen document is already open and it's not possible to open it twice. However duplicating the file and open a copy of the file is possible so you can copy & paste parts of the document." What do you think?
Yep, will do it like that.
Now back on this task. After (hopefully) now having most of the work done in the generic Online code and its iOS-specific parts to have multiple documents open at the same time (disregarding potential performance issues etc), I now then notice that there are lots of problems in the core LibreOffice code when you have multiple documents open in the same LibreOfficeKit-using process. In the (vain) hope that the famous "somebody else" might be interested in having a look and fix it, too, I modified gtktiledviewer to also allow opening multiple documents at the same time: https://gerrit.libreoffice.org/c/core/+/94980 . (It, too, fails badly, and even crashes then.) In retrospect, I should have started with this modification to gtktiledviewer and on making that work.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/bda60c9b1ca6a0fbfde9f3428489f33729cc81f1 tdf#128502: Remove assertions that fire for gtktiledviewer on two docs It will be available in 7.1.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/da68d45ff872cefb8606467c12aedece76a877d7 tdf#128502: Remove one more assertion It will be available in 7.1.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
WIP patch for core at https://gerrit.libreoffice.org/c/core/+/95353
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/79c933916861d085ab844c50d7f07ba9c2d5e570 tdf#128502: Add a FIXME comment It will be available in 7.1.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/a22cba07a470385dcb930c56c90cba81b317773e tdf#128502: Try to support multiple documents in LibreOfficeKit-using process It will be available in 7.1.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/65b5a9848bd01cabbbee8f6e751679a04c1da10f tdf#128502: Avoid potential crash in mobile app with multiple open documents It will be available in 7.1.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/online/commit/7f25109f72738706359a63c9062764699f00f568 tdf#128502: Chunk of work to enable "multi-tasking" in the iOS app
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/dead5cae834e78cacee2275c2d1ca60dac51dd7c tdf#128502: Fix (haha) for a crash with multiple docs open in the iOS app It will be available in 7.1.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
I just tested this with 6.4.0 (16) and it's working quite well now. Cool! @Tor: there are some things I encountered (mainly not rendered tiles in one document) - I'll add a new issue for this. So the question is: do you have any plans regarding this feature (aka: are you still working on it?) or shall we close this one for now? Anyway: thanks a lot to the Collabora team and especiall Tor for taking care of this!
This feature is not something that I would have tested very often, and I am sure there are problems related to it still. But best to file those as separate bugs when encountered. In GitHub. (In fact, I know of a specific problem, that one notices when debugging the app in Xcode but not necessarily as a user of the app, that probably was caused by the work for this bug. I need to file an issue in GitHub for it.)
Resolving this now; let's file follow-up issues in GitHub.