Skip to content

Commit

Permalink
Fix Music app page loading occasionally failing on Chrome
Browse files Browse the repository at this point in the history
It happens sometimes (but quite rarely at least on my tests) that all
the needed parts of the Nextcloud server core are not yet loaded at the
time when our controllers are being initialized. Previously such cases
caused the page loading to fail but now we are prepared: If the needed
parts of the core are not available immediately, then we postpone the
related initialization steps and retry after 500 ms. Attempts are
continued until when they succeed.

In the GitHub issue, the problem was reported to happen with the
reference OC.MimeType, but fixing this case reveled that there was a
similar case related to the HTML element #app-navigation-toggle.

refs #1137
  • Loading branch information
paulijar committed Apr 18, 2024
1 parent 1c839a7 commit 415a26d
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 21 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
- Files player: Menu icon for "Import list to Music" not adjusted correctly for the dark theme
- Standard NC viewer opened instead of embedded Music player when opening file from Dashboard on NC28+
[#1126](https://github.com/owncloud/music/issues/1126)
- Music app page loading randomly failing on Chrome
[#1137](https://github.com/owncloud/music/issues/1137)
- Ampache:
* API not working on ownCloud 10.14.0 (HTTP error 500 on all Ampache API calls)
[#1138](https://github.com/owncloud/music/issues/1138)
Expand Down
12 changes: 8 additions & 4 deletions js/app/controllers/maincontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@ function ($rootScope, $scope, $timeout, $window, ArtistFactory,
// setup dark theme support for Nextcloud versions older than 25
OCA.Music.DarkThemeLegacySupport.applyOnElement(document.getElementById('app'));

// create a global rule to use themed icons for folders everywhere, the default icon-folder is not themed on NC 25 and later
const folderStyle = document.createElement('style');
folderStyle.innerHTML = `#app-view .icon-folder { background-image: url(${OC.MimeType.getIconUrl('dir')}) }`;
document.head.appendChild(folderStyle);
// Create a global rule to use themed icons for folders everywhere, the default icon-folder is not themed on NC 25 and later.
// It happens sometimes (at least on Chrome), that OC.MimeType is not yet present when we come here (see
// https://github.com/owncloud/music/issues/1137). In those cases, we need to postpone registering the folder style.
OCA.Music.Utils.executeOnceRefAvailable(() => OC.MimeType, (ocMimeType) => {
const folderStyle = document.createElement('style');
folderStyle.innerHTML = `#app-view .icon-folder { background-image: url(${ocMimeType.getIconUrl('dir')}) }`;
document.head.appendChild(folderStyle);
});

$rootScope.playing = false;
$rootScope.playingView = null;
Expand Down
38 changes: 23 additions & 15 deletions js/app/controllers/navigationcontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,24 +359,32 @@ angular.module('Music').controller('NavigationController', [

// Dragging an entity over the navigation toggle pops the navigation pane open.
// Subsequently, ending the drag closes the navigation pane.
let navOpenedByDrag = false;
const navToggle = document.getElementById('app-navigation-toggle');
navToggle.addEventListener('dragenter', function() {
if (!navOpenedByDrag) {
navOpenedByDrag = true;
expandCollapsedNavigationPane();
}
});
document.addEventListener('dragend', function() {
if (navOpenedByDrag) {
navOpenedByDrag = false;
$scope.collapseNavigationPaneOnMobile();
// It occasionally happens (at least on Chrome) that the navigation toggle is not yet
// present when this controller is initialized. In those cases, the related logic
// is injected a bit later. See https://github.com/owncloud/music/issues/1137.
OCA.Music.Utils.executeOnceRefAvailable(
() => document.getElementById('app-navigation-toggle'),
(navToggle) => {
let navOpenedByDrag = false;
navToggle.addEventListener('dragenter', () => {
if (!navOpenedByDrag) {
navOpenedByDrag = true;
expandCollapsedNavigationPane();
}
});
document.addEventListener('dragend', () => {
if (navOpenedByDrag) {
navOpenedByDrag = false;
$scope.collapseNavigationPaneOnMobile();
}
});
}
});
);

function expandCollapsedNavigationPane() {
if (!$('body').hasClass('snapjs-left') && $(navToggle).is(':visible')) {
$timeout(() => $(navToggle).trigger('click'));
const $navToggle = $('#app-navigation-toggle');
if (!$('body').hasClass('snapjs-left') && $navToggle.is(':visible')) {
$timeout(() => $navToggle.trigger('click'));
}
}

Expand Down
20 changes: 18 additions & 2 deletions js/shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* later. See the COPYING file.
*
* @author Pauli Järvinen <pauli.jarvinen@gmail.com>
* @copyright Pauli Järvinen 2018 - 2023
* @copyright Pauli Järvinen 2018 - 2024
*/

declare var OCA : any;
Expand Down Expand Up @@ -70,7 +70,23 @@ OCA.Music.Utils = class {
}

/**
* Capitalizes the firts character of the given string
* Attempt to get a reference with the first supplied function. If the reference is available, then call the second
* function giving the reference as an argument. Otherwise retry in a while. Keep trying until the reference is available.
*/
static executeOnceRefAvailable<T>(getRef : () => T|null|undefined, callback : (arg : T) => any, attemptInterval_ms = 500) : void {
const attempt = () => {
let ref = getRef();
if (ref) {
callback(ref);
} else {
setTimeout(attempt, attemptInterval_ms);
}
};
attempt();
}

/**
* Capitalizes the first character of the given string
*/
static capitalize(str : string) : string {
return str && str[0].toUpperCase() + str.slice(1);
Expand Down

0 comments on commit 415a26d

Please sign in to comment.