Fix background removal not applying to user default theming

Signed-off-by: John Molakvoæ <[email protected]>

apps/theming/lib/Themes/CommonThemeTrait.php

@@ -127,15 +127,17 @@ protected function generateUserBackgroundVariables(): array {
 		if ($user !== null
 			&& !$this->themingDefaults->isUserThemingDisabled()
 			&& $this->appManager->isEnabledForUser(Application::APP_ID)) {
+			$adminBackgroundDeleted = $this->config->getAppValue(Application::APP_ID, 'backgroundMime', '') === 'backgroundColor';
 			$backgroundImage = $this->config->getUserValue($user->getUID(), Application::APP_ID, 'background_image', BackgroundService::BACKGROUND_DEFAULT);
 			$currentVersion = (int)$this->config->getUserValue($user->getUID(), Application::APP_ID, 'userCacheBuster', '0');
-			$isPrimaryBright = $this->util->invertTextColor($this->primaryColor);
+			$isPrimaryBright = $this->util->invertTextColor($this->themingDefaults->getColorPrimary());
 
 			// The user removed the background
 			if ($backgroundImage === BackgroundService::BACKGROUND_DISABLED) {
 				return [
-					'--image-background' => 'no',
-					'--color-background-plain' => $this->primaryColor,
+					// Might be defined already by admin theming, needs to be overridden
+					'--image-background' => 'none',
+					'--color-background-plain' => $this->themingDefaults->getColorPrimary(),
 					// If no background image is set, we need to check against the shown primary colour
 					'--background-image-invert-if-bright' => $isPrimaryBright ? 'invert(100%)' : 'no',
 				];
@@ -150,6 +152,15 @@ protected function generateUserBackgroundVariables(): array {
 				];
 			}
 
+			// The user is using the default background and admin removed the background image
+			if ($backgroundImage === BackgroundService::BACKGROUND_DEFAULT && $adminBackgroundDeleted) {
+				return [
+					// --image-background is not defined in this case
+					'--color-background-plain' => $this->themingDefaults->getColorPrimary(),
+					'--background-image-invert-if-bright' => $isPrimaryBright ? 'invert(100%)' : 'no',
+				];
+			}
+
 			// The user picked a shipped background
 			if (isset(BackgroundService::SHIPPED_BACKGROUNDS[$backgroundImage])) {
 				return [

apps/theming/lib/Themes/DefaultTheme.php

@@ -70,7 +70,7 @@ public function __construct(Util $util,
 		$this->defaultPrimaryColor = $this->themingDefaults->getDefaultColorPrimary();
 		$this->primaryColor = $this->themingDefaults->getColorPrimary();
 
-		// Override default defaultPrimaryColor if set to improve accessibility
+		// Override primary colors (if set) to improve accessibility
 		if ($this->primaryColor === BackgroundService::DEFAULT_COLOR) {
 			$this->primaryColor = BackgroundService::DEFAULT_ACCESSIBLE_COLOR;
 		}

apps/theming/lib/ThemingDefaults.php

@@ -255,8 +255,9 @@ public function getColorPrimary(): string {
 	public function getDefaultColorPrimary(): string {
 		$color = $this->config->getAppValue(Application::APP_ID, 'color', '');
 		if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $color)) {
-			$color = '#0082c9';
+			return BackgroundService::DEFAULT_COLOR;
 		}
+
 		return $color;
 	}
 

apps/theming/src/AdminTheming.vue

@@ -297,12 +297,12 @@ export default {
 		/* This is basically https://github.com/nextcloud/server/blob/master/core/css/guest.css
 		   But without the user variables. That way the admin can preview the render as guest*/
 		/* As guest, there is no user color color-background-plain */
-		background-color: var(--color-primary-element-default, #0082c9);
+		background-color: var(--color-primary-default);
 		/* As guest, there is no user background (--image-background)
 		1. Empty background if defined
 		2. Else default background
 		3. Finally default gradient (should not happened, the background is always defined anyway) */
-		background-image: var(--image-background-plain, var(--image-background-default, linear-gradient(40deg, #0082c9 0%, #30b6ff 100%)));
+		background-image: var(--image-background-plain, var(--image-background-default));
 
 		&-logo {
 			width: 20%;

core/css/apps.scss

@@ -48,8 +48,8 @@ html {
 body {
 	// color-background-plain should always be defined. It is the primary user colour
 	background-color: var(--color-background-plain, var(--color-main-background));
-	// image-background-plain means the admin has disabled the background image
-	background-image: var(--image-background, var(--image-background-default));
+	// user background, or plain colour and finally default admin background
+	background-image: var(--image-background, var(--image-background-plain, var(--image-background-default)));
 	background-size: cover;
 	background-position: center;
 	position: fixed;

cypress/e2e/theming/admin-settings.cy.ts

@@ -23,13 +23,10 @@
 import { User } from '@nextcloud/cypress'
 import { colord } from 'colord'
 
-import { pickRandomColor, validateBodyThemingCss, validateUserThemingDefaultCss } from './themingUtils'
+import { defaultPrimary, defaultBackground, pickRandomColor, validateBodyThemingCss, validateUserThemingDefaultCss } from './themingUtils'
 
 const admin = new User('admin', 'admin')
 
-const defaultPrimary = '#0082c9'
-const defaultBackground = 'kamil-porembinski-clouds.jpg'
-
 describe('Admin theming settings visibility check', function() {
 	before(function() {
 		// Just in case previous test failed
@@ -91,7 +88,7 @@ describe('Change the primary color and reset it', function() {
 	})
 })
 
-describe.only('Remove the default background and restore it', function() {
+describe('Remove the default background and restore it', function() {
 	before(function() {
 		// Just in case previous test failed
 		cy.resetAdminTheming()
@@ -109,11 +106,10 @@ describe.only('Remove the default background and restore it', function() {
 		cy.get('[data-admin-theming-setting-file-remove]').click()
 
 		cy.wait('@removeBackground')
+		cy.waitUntil(() => validateBodyThemingCss(defaultPrimary, null))
 		cy.waitUntil(() => cy.window().then((win) => {
-			const currentBackgroundDefault = getComputedStyle(win.document.body).getPropertyValue('--image-background-default')
 			const backgroundPlain = getComputedStyle(win.document.body).getPropertyValue('--image-background-plain')
-			return !currentBackgroundDefault.includes(defaultBackground)
-				&& backgroundPlain !== ''
+			return backgroundPlain !== ''
 		}))
 	})
 
@@ -408,3 +404,54 @@ describe('The user default background settings reflect the admin theming setting
 		cy.waitUntil(() => validateUserThemingDefaultCss(selectedColor, '/apps/theming/image/background?v='))
 	})
 })
+
+describe('The user default background settings reflect the admin theming settings with background removed', function() {
+	before(function() {
+		// Just in case previous test failed
+		cy.resetAdminTheming()
+		cy.login(admin)
+	})
+
+	after(function() {
+		cy.resetAdminTheming()
+	})
+
+	it('See the admin theming section', function() {
+		cy.visit('/settings/admin/theming')
+		cy.get('[data-admin-theming-settings]').scrollIntoView().should('be.visible')
+	})
+
+	it('Remove the default background', function() {
+		cy.intercept('*/apps/theming/ajax/updateStylesheet').as('removeBackground')
+
+		cy.get('[data-admin-theming-setting-file-remove]').click()
+
+		cy.wait('@removeBackground')
+		cy.waitUntil(() => validateBodyThemingCss(defaultPrimary, null))
+	})
+
+	it('Login page should match admin theming settings', function() {
+		cy.logout()
+		cy.visit('/')
+
+		cy.waitUntil(() => validateBodyThemingCss(defaultPrimary, null))
+	})
+
+	it('Login as user', function() {
+		cy.createRandomUser().then((user) => {
+			cy.login(user)
+		})
+	})
+
+	it('See the user background settings', function() {
+		cy.visit('/settings/user/theming')
+		cy.get('[data-user-theming-background-settings]').scrollIntoView().should('be.visible')
+	})
+
+	it('Default user background settings should match admin theming settings', function() {
+		cy.get('[data-user-theming-background-default]').should('be.visible')
+		cy.get('[data-user-theming-background-default]').should('have.class', 'background--active')
+
+		cy.waitUntil(() => validateUserThemingDefaultCss(defaultPrimary, null))
+	})
+})

cypress/e2e/theming/themingUtils.ts

@@ -21,23 +21,29 @@
  */
 import { colord } from 'colord'
 
+export const defaultPrimary = '#0082c9'
+export const defaultAccessiblePrimary = '#006aa3'
+export const defaultBackground = 'kamil-porembinski-clouds.jpg'
+
 /**
  * Validate the current page body css variables
  *
  * @param {string} expectedColor the expected color
  * @param {string|null} expectedBackground the expected background
  */
-export const validateBodyThemingCss = function(expectedColor = '#0082c9', expectedBackground: string|null = 'kamil-porembinski-clouds.jpg') {
+export const validateBodyThemingCss = function(expectedColor = defaultPrimary, expectedBackground: string|null = defaultBackground) {
 	return cy.window().then((win) => {
 		const guestBackgroundColor = getComputedStyle(win.document.body).backgroundColor
 		const guestBackgroundImage = getComputedStyle(win.document.body).backgroundImage
 
-		const isValidBackgroundImage = expectedBackground === null
+		const isValidBackgroundColor = colord(guestBackgroundColor).isEqual(expectedColor)
+		const isValidBackgroundImage = !expectedBackground
 			? guestBackgroundImage === 'none'
 			: guestBackgroundImage.includes(expectedBackground)
 
-		return colord(guestBackgroundColor).isEqual(expectedColor)
-			&& isValidBackgroundImage
+		console.debug({ guestBackgroundColor: colord(guestBackgroundColor).toHex(), guestBackgroundImage, expectedColor, expectedBackground, isValidBackgroundColor, isValidBackgroundImage })
+
+		return isValidBackgroundColor && isValidBackgroundImage
 	})
 }
 
@@ -47,7 +53,7 @@ export const validateBodyThemingCss = function(expectedColor = '#0082c9', expect
  * @param {string} expectedColor the expected color
  * @param {string} expectedBackground the expected background
  */
-export const validateUserThemingDefaultCss = function(expectedColor = '#0082c9', expectedBackground: string|null = 'kamil-porembinski-clouds.jpg') {
+export const validateUserThemingDefaultCss = function(expectedColor = defaultPrimary, expectedBackground: string|null = defaultBackground) {
 	return cy.window().then((win) => {
 		const defaultSelectButton = win.document.querySelector('[data-user-theming-background-default]')
 		const customColorSelectButton = win.document.querySelector('[data-user-theming-background-color]')
@@ -59,9 +65,11 @@ export const validateUserThemingDefaultCss = function(expectedColor = '#0082c9',
 		const defaultOptionBorderColor = getComputedStyle(defaultSelectButton).borderColor
 		const colorPickerOptionColor = getComputedStyle(customColorSelectButton).backgroundColor
 
-		const isValidBackgroundImage = expectedBackground === null
+		const isValidBackgroundImage = !expectedBackground
 			? defaultOptionBackground === 'none'
 			: defaultOptionBackground.includes(expectedBackground)
+
+		console.debug(colord(defaultOptionBorderColor).toHex(), colord(colorPickerOptionColor).toHex(), expectedColor, isValidBackgroundImage)
 
 		return isValidBackgroundImage
 			&& colord(defaultOptionBorderColor).isEqual(expectedColor)

cypress/e2e/theming/user-background.cy.ts

@@ -19,12 +19,10 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
-import { User } from '@nextcloud/cypress'
+import type { User } from '@nextcloud/cypress'
 
-import { pickRandomColor, validateBodyThemingCss } from './themingUtils'
+import { defaultPrimary, defaultBackground, pickRandomColor, validateBodyThemingCss } from './themingUtils'
 
-const defaultPrimary = '#006aa3'
-const defaultBackground = 'kamil-porembinski-clouds.jpg'
 const admin = new User('admin', 'admin')
 
 describe('User default background settings', function() {
@@ -85,7 +83,7 @@ describe('User select shipped backgrounds and remove background', function() {
 
 		// Validate changed background and primary
 		cy.wait('@setBackground')
-		cy.waitUntil(() => validateBodyThemingCss('#56633d', background, true))
+		cy.waitUntil(() => validateBodyThemingCss('#56633d', background))
 	})
 
 	it('Remove background', function() {
@@ -121,7 +119,7 @@ describe('User select a custom color', function() {
 		cy.wait('@setColor')
 		cy.waitUntil(() => cy.window().then((win) => {
 			const primary = getComputedStyle(win.document.body).getPropertyValue('--color-primary')
-			return primary !== defaultPrimary
+			return primary !== defaultPrimary && primary !== defaultPrimary
 		}))
 	})
 })
@@ -170,7 +168,7 @@ describe('User select a bright custom color and remove background', function() {
 		}))
 	})
 
-	it('Select a shipped background', function() {
+	it('Select another but non-bright shipped background', function() {
 		const background = 'anatoly-mikhaltsov-butterfly-wing-scale.jpg'
 		cy.intercept('*/apps/theming/background/shipped').as('setBackground')
 
@@ -182,7 +180,7 @@ describe('User select a bright custom color and remove background', function() {
 		cy.waitUntil(() => validateBodyThemingCss('#a53c17', background))
 	})
 
-	it('See the header NOT being inverted', function() {
+	it('See the header NOT being inverted this time', function() {
 		cy.waitUntil(() => cy.window().then((win) => {
 			const firstEntry = win.document.querySelector('.app-menu-main li')
 			if (!firstEntry) {