New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Theme filetype icons applied only intermittently to external folders #25461

Closed
tipichris opened this Issue Jul 12, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@tipichris

tipichris commented Jul 12, 2016

Steps to reproduce

  1. Create a theme with alternative icons for filetypes
  2. Configure OC to use it
  3. Run occ maintenance:mimetype:update-js
  4. View a file list

Expected behaviour

File icons should be those from the theme

Actual behaviour

Most file icons are those from the theme. However, those for external folders are from core.
In fact, the new icon seems to display momentarily, then the row is greyed out briefly and when it is un-greyed the icon is that from core. Various changes that result in ajax calls also result in the icon being changed to that from the theme, for example, marking or unmarking the folder as a favourite.

It appears that some sort of dom manipulation is going on for external folders shortly after the file list loads and this is not respecting the theme. Further dom manipulation does respect the theme and changes the value of the elements background URL accordingly.

Server configuration

Operating system:
Ubuntu 14.04
Web server:
Apache
Database:
MySQL
PHP version:
5.5.9
ownCloud version: (see ownCloud admin page)
9.0.3
Updated from an older ownCloud or fresh install:
Fresh install
Where did you install ownCloud from:
Repositories
Signing status (ownCloud 9.0 and above):
No errors have been found.

List of activated apps:
Enabled:

  • activity: 2.2.1
  • comments: 0.2
  • dav: 0.1.6
  • federatedfilesharing: 0.1.0
  • federation: 0.0.4
  • files: 1.4.4
  • files_antivirus: 0.8.0.2
  • files_external: 0.5.2
  • files_pdfviewer: 0.8.1
  • files_sharing: 0.9.1
  • files_texteditor: 2.1
  • files_trashbin: 0.8.0
  • files_versions: 1.2.0
  • files_videoplayer: 0.9.8
  • firstrunwizard: 1.1
  • gallery: 14.5.0
  • notifications: 0.2.3
  • provisioning_api: 0.4.1
  • systemtags: 0.2
  • templateeditor: 0.1
  • updatenotification: 0.1.0
  • user_ldap: 0.8.0
    Disabled:
  • encryption
  • external
  • user_external

The content of config/config.php:
$CONFIG = array (
'updatechecker' => false,
'instanceid' => 'REMOVED',
'passwordsalt' => 'REMOVED,
'secret' => 'REMOVED',
'trusted_domains' =>
array (
0 => 'fileshare.REMOVED.com',
),
'datadirectory' => '/var/ownclouddata',
'overwrite.cli.url' => 'https://fileshare.REMOVED/owncloud',
'dbtype' => 'mysql',
'version' => '9.0.3.2',
'dbname' => 'owncloud',
'dbhost' => 'localhost',
'dbtableprefix' => 'oc_',
'dbuser' => 'oc_admin',
'dbpassword' => 'REMOVED',
'logtimezone' => 'UTC',
'installed' => true,
'ldapIgnoreNamingRules' => false,
'mail_smtpmode' => 'php',
'mail_from_address' => 'info',
'mail_domain' => 'REMOVED.com',
'theme' => 'aspect',
'loglevel' => 0,
'memcache.local' => '\OC\Memcache\APCu',
);
Are you using external storage, if yes which one: local/smb/sftp/...
SMB
Are you using encryption: yes/no
No
Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...
Yes, LDAP

LDAP configuration (delete this part if not used)

Sorry, it's big and unlikely to be relevant

Client configuration

Browser:
Firefox
Operating system:
Ubuntu

@PVince81 PVince81 added this to the 9.2 milestone Jul 13, 2016

@PVince81

This comment has been minimized.

Show comment
Hide comment
@PVince81

PVince81 Jul 13, 2016

Member

If you see it appearing and being replaced, I suspect it's the "external storage availability check" code that replaces it with the wrong icon and doesn't use the theme.

@jvillafanez

Member

PVince81 commented Jul 13, 2016

If you see it appearing and being replaced, I suspect it's the "external storage availability check" code that replaces it with the wrong icon and doesn't use the theme.

@jvillafanez

@tipichris

This comment has been minimized.

Show comment
Hide comment
@tipichris

tipichris Jul 13, 2016

It looks like the problem lies in OCA.External.StatusManager.Utils.getIconRoute

This patch sorts the immediate problem out for me, but it looks as if there would still be problems sharepoint and Windows Network Drives
external-folder-theme-icon-bug-patch.txt

tipichris commented Jul 13, 2016

It looks like the problem lies in OCA.External.StatusManager.Utils.getIconRoute

This patch sorts the immediate problem out for me, but it looks as if there would still be problems sharepoint and Windows Network Drives
external-folder-theme-icon-bug-patch.txt

@jvillafanez

This comment has been minimized.

Show comment
Hide comment
@jvillafanez

jvillafanez Jul 15, 2016

Member

The code isn't the same as the one for 8.2, and that part is something that changed between versions, so I'm not going to be of help here...

Anyway, the patch make sense. The only thing "weird" is the name of the icon. I'm not sure if the one from the theming should match with the default one (dir-external -> folder-external).
Regarding sharepoint and WND icons, I'm not sure if they're themeables, but there shouldn't be any change with the patch anyway.

@tipichris could you make a PR with the patch so we can review and test it?

Member

jvillafanez commented Jul 15, 2016

The code isn't the same as the one for 8.2, and that part is something that changed between versions, so I'm not going to be of help here...

Anyway, the patch make sense. The only thing "weird" is the name of the icon. I'm not sure if the one from the theming should match with the default one (dir-external -> folder-external).
Regarding sharepoint and WND icons, I'm not sure if they're themeables, but there shouldn't be any change with the patch anyway.

@tipichris could you make a PR with the patch so we can review and test it?

tipichris added a commit to tipichris/core that referenced this issue Jul 15, 2016

Respect theme for external folder icon
See issue owncloud#25461. 
When using a theme with alternative filetype icons `OCA.External.StatusManager.Utils.getIconRoute` should respect that, rather than hard coding the default icon.

Note this change does not affect windows_network_drive and sharepoint icons, which appear not to be easily themeable.
@tipichris

This comment has been minimized.

Show comment
Hide comment
@tipichris

tipichris Jul 15, 2016

@jvillafanez PR done. dir-external is not the name of the icon. I guess you could call it a pseudo-mimetype. It gets fed to OC.MimeType._getFile which returns the icon file name, folder-external.

tipichris commented Jul 15, 2016

@jvillafanez PR done. dir-external is not the name of the icon. I guess you could call it a pseudo-mimetype. It gets fed to OC.MimeType._getFile which returns the icon file name, folder-external.

DeepDiver1975 added a commit that referenced this issue Aug 5, 2016

Respect theme for external folder icon (#25487)
See issue #25461. 
When using a theme with alternative filetype icons `OCA.External.StatusManager.Utils.getIconRoute` should respect that, rather than hard coding the default icon.

Note this change does not affect windows_network_drive and sharepoint icons, which appear not to be easily themeable.

VicDeo added a commit that referenced this issue Aug 5, 2016

Respect theme for external folder icon (#25487)
See issue #25461. 
When using a theme with alternative filetype icons `OCA.External.StatusManager.Utils.getIconRoute` should respect that, rather than hard coding the default icon.

Note this change does not affect windows_network_drive and sharepoint icons, which appear not to be easily themeable.

VicDeo added a commit that referenced this issue Aug 5, 2016

Respect theme for external folder icon (#25487)
See issue #25461. 
When using a theme with alternative filetype icons `OCA.External.StatusManager.Utils.getIconRoute` should respect that, rather than hard coding the default icon.

Note this change does not affect windows_network_drive and sharepoint icons, which appear not to be easily themeable.

DeepDiver1975 added a commit that referenced this issue Aug 9, 2016

Respect theme for external folder icon (#25487) (#25712)
See issue #25461. 
When using a theme with alternative filetype icons `OCA.External.StatusManager.Utils.getIconRoute` should respect that, rather than hard coding the default icon.

Note this change does not affect windows_network_drive and sharepoint icons, which appear not to be easily themeable.

DeepDiver1975 added a commit that referenced this issue Aug 9, 2016

Respect theme for external folder icon (#25487) (#25711)
See issue #25461. 
When using a theme with alternative filetype icons `OCA.External.StatusManager.Utils.getIconRoute` should respect that, rather than hard coding the default icon.

Note this change does not affect windows_network_drive and sharepoint icons, which appear not to be easily themeable.

@PVince81 PVince81 closed this Dec 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment