diff --git a/core/js/jquery.avatar.js b/core/js/jquery.avatar.js index 54518c75cc..958f0f9edd 100644 --- a/core/js/jquery.avatar.js +++ b/core/js/jquery.avatar.js @@ -106,54 +106,39 @@ }); } - // If the displayname is not defined we use the old code path - if (typeof(displayname) === 'undefined') { - $.get(url).always(function(result, status) { - // if there is an error or an object returned (contains user information): - // -> show the fallback placeholder - if (typeof(result) === 'object' || status === 'error') { - if (!hidedefault) { - if (result.data && result.data.displayname) { - $div.imageplaceholder(user, result.data.displayname); - } else { - // User does not exist - setAvatarForUnknownUser($div); - } - } else { - $div.hide(); - } - // else an image is transferred and should be shown - } else { - $div.show(); - if (ie8fix === true) { - $div.html(''); - } else { - $div.html(''); - } - } - if(typeof callback === 'function') { - callback(); - } - }); - } else { - // We already have the displayname so set the placeholder (to show at least something) - if (!hidedefault) { - $div.imageplaceholder(displayname); + var img = new Image(); + + // If the new image loads successfully set it. + img.onload = function() { + $div.text(''); + $div.append(img); + $div.clearimageplaceholder(); + + if(typeof callback === 'function') { + callback(); + } + }; + // Fallback when avatar loading fails: + // Use old placeholder when a displayname attribute is defined, + // otherwise show the unknown user placeholder. + img.onerror = function () { + $div.clearimageplaceholder(); + if (typeof(displayname) !== 'undefined') { + $div.imageplaceholder(user, displayname); + } else { + setAvatarForUnknownUser($div); + $div.removeClass('icon-loading'); } - var img = new Image(); + if(typeof callback === 'function') { + callback(); + } + }; - // If the new image loads successfully set it. - img.onload = function() { - $div.show(); - $div.text(''); - $div.append(img); - $div.clearimageplaceholder(); - }; - - img.width = size; - img.height = size; - img.src = url; - } + $div.addClass('icon-loading'); + $div.show(); + img.width = size; + img.height = size; + img.src = url; }; }(jQuery)); diff --git a/core/js/placeholder.js b/core/js/placeholder.js index f173e73867..5cf7b9095a 100644 --- a/core/js/placeholder.js +++ b/core/js/placeholder.js @@ -2,7 +2,7 @@ * ownCloud * * @author John Molakvoæ - * @copyright 2016 John Molakvoæ + * @copyright 2016-2017 John Molakvoæ * @author Morris Jobke * @copyright 2013 Morris Jobke * @@ -47,7 +47,7 @@ *
A
* */ - + /* * Alternatively, you can use the prototype function to convert your string to hsl colors: * @@ -156,5 +156,6 @@ this.css('text-align', ''); this.css('line-height', ''); this.css('font-size', ''); + this.removeClass('icon-loading'); }; }(jQuery)); diff --git a/core/js/tests/specs/jquery.avatarSpec.js b/core/js/tests/specs/jquery.avatarSpec.js index b9351d2a8a..bdd1fdcc16 100644 --- a/core/js/tests/specs/jquery.avatarSpec.js +++ b/core/js/tests/specs/jquery.avatarSpec.js @@ -19,6 +19,13 @@ describe('jquery.avatar tests', function() { devicePixelRatio = window.devicePixelRatio; window.devicePixelRatio = 1; + + spyOn(window, 'Image').and.returnValue({ + onload: function() { + }, + onerror: function() { + } + }); }); afterEach(function() { @@ -39,6 +46,9 @@ describe('jquery.avatar tests', function() { $div.height(9); $div.avatar('foo'); + expect(window.Image).toHaveBeenCalled(); + window.Image().onerror(); + expect($div.height()).toEqual(9); expect($div.width()).toEqual(9); }); @@ -47,6 +57,9 @@ describe('jquery.avatar tests', function() { $div.data('size', 10); $div.avatar('foo'); + expect(window.Image).toHaveBeenCalled(); + window.Image().onerror(); + expect($div.height()).toEqual(10); expect($div.width()).toEqual(10); }); @@ -55,6 +68,9 @@ describe('jquery.avatar tests', function() { it('defined', function() { $div.avatar('foo', 8); + expect(window.Image).toHaveBeenCalled(); + window.Image().onerror(); + expect($div.height()).toEqual(8); expect($div.width()).toEqual(8); }); @@ -73,16 +89,10 @@ describe('jquery.avatar tests', function() { describe('no avatar', function() { it('show placeholder for existing user', function() { spyOn($div, 'imageplaceholder'); - $div.avatar('foo'); - - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'application/json' }, - JSON.stringify({ - data: {displayname: 'bar'} - }) - ); + $div.avatar('foo', undefined, undefined, undefined, undefined, 'bar'); + expect(window.Image).toHaveBeenCalled(); + window.Image().onerror(); expect($div.imageplaceholder).toHaveBeenCalledWith('foo', 'bar'); }); @@ -91,32 +101,23 @@ describe('jquery.avatar tests', function() { spyOn($div, 'css'); $div.avatar('foo'); - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'application/json' }, - JSON.stringify({ - data: {} - }) - ); + expect(window.Image).toHaveBeenCalled(); + window.Image().onerror(); expect($div.imageplaceholder).toHaveBeenCalledWith('?'); expect($div.css).toHaveBeenCalledWith('background-color', '#b9b9b9'); }); - it('show no placeholder', function() { + it('show no placeholder is ignored', function() { spyOn($div, 'imageplaceholder'); + spyOn($div, 'css'); $div.avatar('foo', undefined, undefined, true); - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'application/json' }, - JSON.stringify({ - data: {} - }) - ); + expect(window.Image).toHaveBeenCalled(); + window.Image().onerror(); - expect($div.imageplaceholder.calls.any()).toEqual(false); - expect($div.css('display')).toEqual('none'); + expect($div.imageplaceholder).toHaveBeenCalledWith('?'); + expect($div.css).toHaveBeenCalledWith('background-color', '#b9b9b9'); }); }); @@ -129,24 +130,24 @@ describe('jquery.avatar tests', function() { window.devicePixelRatio = 1; $div.avatar('foo', 32); - expect(fakeServer.requests[0].method).toEqual('GET'); - expect(fakeServer.requests[0].url).toEqual('http://localhost/index.php/avatar/foo/32'); + expect(window.Image).toHaveBeenCalled(); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/32'); }); it('high DPI icon', function() { window.devicePixelRatio = 4; $div.avatar('foo', 32); - expect(fakeServer.requests[0].method).toEqual('GET'); - expect(fakeServer.requests[0].url).toEqual('http://localhost/index.php/avatar/foo/128'); + expect(window.Image).toHaveBeenCalled(); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/128'); }); it('high DPI icon round up size', function() { window.devicePixelRatio = 1.9; $div.avatar('foo', 32); - expect(fakeServer.requests[0].method).toEqual('GET'); - expect(fakeServer.requests[0].url).toEqual('http://localhost/index.php/avatar/foo/61'); + expect(window.Image).toHaveBeenCalled(); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/61'); }); }); @@ -158,17 +159,12 @@ describe('jquery.avatar tests', function() { it('default (no ie8 fix)', function() { $div.avatar('foo', 32); - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'image/jpeg' }, - '' - ); + expect(window.Image).toHaveBeenCalled(); + window.Image().onload(); - var img = $div.children('img')[0]; - - expect(img.height).toEqual(32); - expect(img.width).toEqual(32); - expect(img.src).toEqual('http://localhost/index.php/avatar/foo/32'); + expect(window.Image().height).toEqual(32); + expect(window.Image().width).toEqual(32); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/32'); }); it('default high DPI icon', function() { @@ -176,37 +172,23 @@ describe('jquery.avatar tests', function() { $div.avatar('foo', 32); - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'image/jpeg' }, - '' - ); + expect(window.Image).toHaveBeenCalled(); + window.Image().onload(); - var img = $div.children('img')[0]; - - expect(img.height).toEqual(32); - expect(img.width).toEqual(32); - expect(img.src).toEqual('http://localhost/index.php/avatar/foo/61'); + expect(window.Image().height).toEqual(32); + expect(window.Image().width).toEqual(32); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/61'); }); - it('with ie8 fix', function() { - sinon.stub(Math, 'random').callsFake(function() { - return 0.5; - }); - + it('with ie8 fix (ignored)', function() { $div.avatar('foo', 32, true); - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'image/jpeg' }, - '' - ); + expect(window.Image).toHaveBeenCalled(); + window.Image().onload(); - var img = $div.children('img')[0]; - - expect(img.height).toEqual(32); - expect(img.width).toEqual(32); - expect(img.src).toEqual('http://localhost/index.php/avatar/foo/32#500'); + expect(window.Image().height).toEqual(32); + expect(window.Image().width).toEqual(32); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/32'); }); it('unhide div', function() { @@ -214,11 +196,12 @@ describe('jquery.avatar tests', function() { $div.avatar('foo', 32); - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'image/jpeg' }, - '' - ); + expect(window.Image).toHaveBeenCalled(); + window.Image().onload(); + + expect(window.Image().height).toEqual(32); + expect(window.Image().width).toEqual(32); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/32'); expect($div.css('display')).toEqual('block'); }); @@ -232,12 +215,12 @@ describe('jquery.avatar tests', function() { observer.callback(); }); - fakeServer.requests[0].respond( - 200, - { 'Content-Type': 'image/jpeg' }, - '' - ); + expect(window.Image).toHaveBeenCalled(); + window.Image().onload(); + expect(window.Image().height).toEqual(32); + expect(window.Image().width).toEqual(32); + expect(window.Image().src).toEqual('http://localhost/index.php/avatar/foo/32'); expect(observer.callback).toHaveBeenCalled(); }); }); diff --git a/lib/private/Avatar.php b/lib/private/Avatar.php index 5893daa180..afa9118c50 100644 --- a/lib/private/Avatar.php +++ b/lib/private/Avatar.php @@ -141,6 +141,7 @@ class Avatar implements IAvatar { try { $generated = $this->folder->getFile('generated'); + $this->config->setUserValue($this->user->getUID(), 'avatar', 'generated', 'false'); $generated->delete(); } catch (NotFoundException $e) { // @@ -161,6 +162,7 @@ class Avatar implements IAvatar { foreach ($avatars as $avatar) { $avatar->delete(); } + $this->config->setUserValue($this->user->getUID(), 'avatar', 'generated', 'true'); $this->user->triggerChange('avatar', ''); } @@ -177,6 +179,7 @@ class Avatar implements IAvatar { $ext = 'png'; $this->folder->newFile('generated'); + $this->config->setUserValue($this->user->getUID(), 'avatar', 'generated', 'true'); } if ($size === -1) { @@ -393,4 +396,18 @@ class Avatar implements IAvatar { return array(round($r * 255), round($g * 255), round($b * 255)); } + public function userChanged($feature, $oldValue, $newValue) { + // We only change the avatar on display name changes + if ($feature !== 'displayName') { + return; + } + + // If the avatar is not generated (so an uploaded image) we skip this + if (!$this->folder->fileExists('generated')) { + return; + } + + $this->remove(); + } + } diff --git a/lib/private/Server.php b/lib/private/Server.php index 44f5ea80cb..4a851d6722 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -394,9 +394,10 @@ class Server extends ServerContainer implements IServerContainer { $userSession->listen('\OC\User', 'logout', function () { \OC_Hook::emit('OC_User', 'logout', array()); }); - $userSession->listen('\OC\User', 'changeUser', function ($user, $feature, $value, $oldValue) { + $userSession->listen('\OC\User', 'changeUser', function ($user, $feature, $value, $oldValue) use ($dispatcher) { /** @var $user \OC\User\User */ \OC_Hook::emit('OC_User', 'changeUser', array('run' => true, 'user' => $user, 'feature' => $feature, 'value' => $value, 'old_value' => $oldValue)); + $dispatcher->dispatch('OCP\IUser::changeUser', new GenericEvent($user, ['feature' => $feature, 'oldValue' => $oldValue, 'value' => $value])); }); return $userSession; }); @@ -1175,6 +1176,22 @@ class Server extends ServerContainer implements IServerContainer { $logger->info('Could not cleanup avatar of ' . $user->getUID()); } }); + + $dispatcher->addListener('OCP\IUser::changeUser', function (GenericEvent $e) { + $manager = $this->getAvatarManager(); + /** @var IUser $user */ + $user = $e->getSubject(); + $feature = $e->getArgument('feature'); + $oldValue = $e->getArgument('oldValue'); + $value = $e->getArgument('value'); + + try { + $avatar = $manager->getAvatar($user->getUID()); + $avatar->userChanged($feature, $oldValue, $value); + } catch (NotFoundException $e) { + // no avatar to remove + } + }); } /** diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index 60ac4bfecb..551fc3b9b0 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -254,6 +254,7 @@ class JSConfigHelper { $array['oc_userconfig'] = json_encode([ 'avatar' => [ 'version' => (int)$this->config->getUserValue($uid, 'avatar', 'version', 0), + 'generated' => $this->config->getUserValue($uid, 'avatar', 'generated', 'true') === 'true', ] ]); } diff --git a/lib/public/IAvatar.php b/lib/public/IAvatar.php index 369cafa00c..a6731b63be 100644 --- a/lib/public/IAvatar.php +++ b/lib/public/IAvatar.php @@ -77,4 +77,10 @@ interface IAvatar { * @since 9.0.0 */ public function getFile($size); + + /** + * Handle a changed user + * @since 13.0.0 + */ + public function userChanged($feature, $oldValue, $newValue); } diff --git a/settings/js/settings/personalInfo.js b/settings/js/settings/personalInfo.js index 3a4542df74..0a39e60776 100644 --- a/settings/js/settings/personalInfo.js +++ b/settings/js/settings/personalInfo.js @@ -60,7 +60,7 @@ function updateAvatar (hidedefault) { $displaydiv.avatar(user.uid, 145, true, null, function() { $displaydiv.removeClass('loading'); $('#displayavatar img').show(); - if($('#displayavatar img').length === 0) { + if($('#displayavatar img').length === 0 || oc_userconfig.avatar.generated) { $('#removeavatar').removeClass('inlineblock').addClass('hidden'); } else { $('#removeavatar').removeClass('hidden').addClass('inlineblock'); @@ -129,6 +129,7 @@ function avatarResponseHandler (data) { $warning.hide(); if (data.status === "success") { $('#displayavatar .avatardiv').removeClass('icon-loading'); + oc_userconfig.avatar.generated = false; updateAvatar(); } else if (data.data === "notsquare") { showAvatarCropper(); @@ -256,8 +257,14 @@ $(document).ready(function () { }); + var userSettings = new OC.Settings.UserSettings(); var federationSettingsView = new OC.Settings.FederationSettingsView({ - el: '#personal-settings' + el: '#personal-settings', + config: userSettings + }); + + userSettings.on("sync", function() { + updateAvatar(false); }); federationSettingsView.render(); @@ -362,6 +369,7 @@ $(document).ready(function () { type: 'DELETE', url: OC.generateUrl('/avatar/'), success: function () { + oc_userconfig.avatar.generated = true; updateAvatar(true); } }); @@ -392,7 +400,7 @@ $(document).ready(function () { // Load the big avatar var user = OC.getCurrentUser(); $('#avatarform .avatardiv').avatar(user.uid, 145, true, null, function() { - if($('#displayavatar img').length === 0) { + if($('#displayavatar img').length === 0 || oc_userconfig.avatar.generated) { $('#removeavatar').removeClass('inlineblock').addClass('hidden'); } else { $('#removeavatar').removeClass('hidden').addClass('inlineblock'); diff --git a/tests/lib/AvatarTest.php b/tests/lib/AvatarTest.php index 240aecc115..9da719c26d 100644 --- a/tests/lib/AvatarTest.php +++ b/tests/lib/AvatarTest.php @@ -210,7 +210,7 @@ class AvatarTest extends \Test\TestCase { ->method('putContent') ->with($image->data()); - $this->config->expects($this->once()) + $this->config->expects($this->exactly(3)) ->method('setUserValue'); $this->config->expects($this->once()) ->method('getUserValue');