Add favicon for IE8-10

[MorrisJobke]

cc @jancborchardt

Fixes #18012

This doesn't work yet and needs further refinement. This is my current state of the fix.

[scrutinizer-notifier]

A new inspection was created.

[jancborchardt]

@CaptionF please review if this fixes your issue.

[MorrisJobke]

@jancborchardt please read my comments. This is not yet finished and therefore also marked as wip

[MorrisJobke]

@jancborchardt For favicon support in IE we need to generate ico files. For older IEs (8, 9) we also need to place them in the root of the URL (https://my.domain.com/favicon.ico - could be done via rewrite rule if owncloud is in a subfolder) - for higher version it can also be placed in a subdirectory (https://my.domain.com/owncloud/favicon.ico is valid).

@MTRichards Is this worth the effort?

[jancborchardt]

@MTRichards @DeepDiver1975 statement please if this is worth the effort.

[MTRichards]

Yes please, because it is IE up to 10.

[MorrisJobke]

Yes please, because it is IE up to 10.

Okay. So this PR already solves it for IE10. For IE8 & 9 it will only work if owncloud is installed in the root and not a subdirectory and we add some rewrite rules. 🙈

[DeepDiver1975]

@MorrisJobke let's move this one in - fixes at least some case.

[DeepDiver1975]

@rperezb we need qa power on this - thx

[MorrisJobke]

I just downloaded the IE10 image to test if it even fixes it in IE10

[MorrisJobke]

I tested the latest changes and they now work with IE10

I will try to get it working in IE9 ...but I think this is harder.

[MorrisJobke]

I need to choose an ico file to support IE9 and IE10. PNG support started with IE11: https://en.wikipedia.org/wiki/Favicon#How_to_use

I tested this and it works in IE8, IE9 and IE10 too now.

Also work in other browsers - Firefox: (left old version, right, new version)

@DeepDiver1975 Ready for review now.

[MorrisJobke]

I also updated the example theme favicon.

[MorrisJobke]

@LukasReschke @nickvergessen @CaptionF Please test

[PVince81]

Moving to 9.0 to match the ticket

[davitol]

@MorrisJobke 👎 It doesn't work for me. 😿

logs:

{"reqId":"K9nO0WLyPgk+SP84WiDs","remoteAddr":"82.159.139.58","app":"core","message":"Exception: {"Exception":"RuntimeException","Message":"image not found: image:favicon.ico webroot: serverroot:\/opt\/owncloud","Code":0,"Trace":"#0 \/opt\/owncloud\/lib\/private\/helper.php(174): OC\URLGenerator->imagePath('', 'favicon.ico')\n#1 \/opt\/owncloud\/lib\/private\/template\/functions.php(169): OC_Helper::imagePath('', 'favicon.ico')\n#2 \/opt\/owncloud\/core\/templates\/layout.guest.php(15): image_path('', 'favicon.ico')\n#3 \/opt\/owncloud\/lib\/private\/template\/base.php(160): include('\/opt\/owncloud\/c...')\n#4 \/opt\/owncloud\/lib\/private\/template\/base.php(138): OC\Template\Base->load('\/opt\/owncloud\/c...')\n#5 \/opt\/owncloud\/lib\/private\/template.php(211): OC\Template\Base->fetchPage()\n#6 \/opt\/owncloud\/lib\/private\/template.php(233): OC_Template->fetchPage()\n#7 \/opt\/owncloud\/lib\/private\/template\/base.php(121): OC_Template->fetchPage()\n#8 \/opt\/owncloud\/lib\/private\/template.php(338): OC\Template\Base->printPage()\n#9 \/opt\/owncloud\/index.php(55): OC_Template::printExceptionErrorPage(Object(RuntimeException))\n#10 {main}","File":"\/opt\/owncloud\/lib\/private\/urlgenerator.php","Line":189}","level":3,"time":"2015-10-13T08:59:30+00:00"}

[MorrisJobke]

@MorrisJobke It doesn't work for me.

Which browser? Which OS?

[davitol]

@MorrisJobke IE9, WIn XP (64-bits) ; sorry about not writing the info in the previous comment

[MorrisJobke]

@MorrisJobke IE9, WIn XP (64-bits) ; sorry about not writing the info in the previous comment

Can you check that you use the correct commit 457b43f ? I rebased recently, maybe you are on an old version 🙈

[PVince81]

@davitol any update ?

[davitol]

Can you check that you use the correct commit 457b43f

@PVince81 @MorrisJobke I have retested and it doesn't work for me. Same error as said before

[MorrisJobke]

@PVince81 @MorrisJobke I have retested and it doesn't work for me. Same error as said before

I don't get it. I just rebased this again. Maybe we can debug this together at your machine via screensharing? I would like to get this in. Just ping me once you have time @davitol

[davitol]

👍 Now it WFM @MorrisJobke

[MorrisJobke]

@PVince81 Mind to give this a short test?

[PVince81]

Tested IE8, IE9, IE10, IE11, Firefox and Chrome, combined with login page, public link and regular page => favicon works 👍

[mmccarn]

The favicon.ico patch seems not to work with a custom theme:
favicon.ico from custom theme is not applied #23758

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.