Feature - Implemented add favicon back to relay browser

I

IronTetsubo

Member
I use chrome (and the same is true in firefox, no idea in IE), and I had been (very slightly :)) annoyed for a while because I have a ton of tabs open, to the point where each tab is only represented by it's favicon. I know, I should use less tabs, but even still I poked around and found that a loooong time ago (2007!) - shwei removed favicon explicitly - http://kolmafia.svn.sourceforge.net/viewvc/kolmafia?view=revision&revision=3790. Unfortunately, my perusal of the forums turned up no info as to why, and I'm hoping that times have changed and this is a useful feature. It is for me - before using kolmafia I got used to seeing the little martini glass in my tabs, and I'd prefer to have it back. I thought it would be a one-line fix, but it turned out to be slightly more complicated, see attached patch. If there's a preference here on how I handled it, or you'd prefer a more optimized route where it gets cached more aggressively, let me know, happy to rework the patch. As always, patch is off of svn head (r11267), builds/compiles/works.
 

Attachments

  • favicon.patch
    1.4 KB · Views: 51
B

bordemstirs

Member
Odd, I haven't even been playing KoL for 5 years (and less time with Mafia) but I remember it having a favicon before, and noticed that it went missing a while ago. I think when I started using jars instead of the exe-wrapped builds? Or I just have a REALLY shitty attention and it went away when I started using mafia... which seems far more likely. Anyway, +1 to this request.
 
Bale

Bale

Minion
IronTetsubo said:
I poked around and found that a loooong time ago (2007!) - shwei removed favicon explicitly - http://kolmafia.svn.sourceforge.net/viewvc/kolmafia?view=revision&revision=3790. Unfortunately, my perusal of the forums turned up no info as to why
Click to expand...

I cannot remember the details, but I have a faint memory that this was because it was causing a problem on Linux whose cause and solution were unclear, so Hola decided to cut the Gordian Knot and simply remove the favicon since it seemed unimportant. That's what I remember, but I make no promise of accuracy.
 
matt.chugg

matt.chugg

Moderator
I think that the relay browser should have a different Icon to vanilla KoL, could be a useful way to know which one you are logged into (not that I ever user vanilla kol)

I also think it should be served up from the relay folder using a link tag inserted into the html <head>, instead of relaying on the browser to hit the standard favicon.ico in the root and just assuming its there. Since kol relies on the browser to hit favicon in the root, including a proper link tag would prevent random hits to favicon,ico.

I think RelayRequest.java could be modified to include

Code: 
this.responseText = StringUtilities.singleStringReplace(this.responseText,"<head>", "<head><link rel=\"icon\" type=\"image/png\" href=\"http://127.0.0.1:" + RelayServer.getPort() + "/myicon.png\" />");

or something similar (would need a png included in the right place as well and maybe?? another override to allow the request to the png through. unless the png goes in the images folder?? which is perfectly acceptable
 
Last edited:
I

IronTetsubo

Member
slyz said:
I tried the patch, but I'm not seeing the image in FF 13.0.1. The image for the tab is simply an empty dashed-line square.

I haven't tried with chrome, but I made sure that I see the image when going to http://www.kingdomofloathing.com/favicon.ico.
Click to expand...

Eh? Well, that's strange. After starting your relay browser, what do you get when going to "http://127.0.0.1:60080/favicon.ico"? (or whatever your relay server address/port is, of course) Maybe I missed a part of the patch when doing the diff...

More broadly to the points about "kolmafia should be different than kol's favicon", I'm all good with that, but I'm not an artist. :) So, this is better than the difference being no icon at all. I'm not sure about served from the relay folder, catch-22? Are you suggesting we cache a local copy in the relay folder?
 
C

Catch-22

Active member
IronTetsubo said:
I'm not sure about served from the relay folder, catch-22? Are you suggesting we cache a local copy in the relay folder?
Click to expand...

Umm well, not exactly. We just store it in the relay as part of KoLmafia's default files. That way people can override it if they want à la matt.chugg, who describes pretty much what I envisage also.
 
S

slyz

Developer
It ended up working while I was playing today. The ten first times I opened the realy browser, nothing, but now I have it.
 
I

IronTetsubo

Member
slyz said:
It ended up working while I was playing today. The ten first times I opened the realy browser, nothing, but now I have it.
Click to expand...

Shrug. Okay - well, if you can duplicate the issue, let me know what response you get when going there via the relay browser, happy to tweak the patch as necessary.
 
matt.chugg

matt.chugg

Moderator
slyz said:
It ended up working while I was playing today. The ten first times I opened the realy browser, nothing, but now I have it.
Click to expand...

Chrome has refresh issues with things like this, it doesn't have a proper Hard Refresh (ctrl-f5 on FF) and quite often caches icons and stylesheets, dumping your cache would probably force it to redownload the icon, or a new private browsing tab
 
Theraze

Theraze

Active member
Realistically, for it to be committable, with the removal being to solve a troublesome linux bug... Either a preponderance of developers who use linux chiming in and saying it's working, or hola himself with the specifics of the 5-6 year old bug. Or someone who just doesn't care. :) Any of those are likely to get it committed quickly. ;)
 
I

IronTetsubo

Member
Theraze said:
Realistically, for it to be committable, with the removal being to solve a troublesome linux bug... Either a preponderance of developers who use linux chiming in and saying it's working, or hola himself with the specifics of the 5-6 year old bug. Or someone who just doesn't care. :) Any of those are likely to get it committed quickly. ;)
Click to expand...

Heh, fair enough - I should have explained further. I'm primarily a linux user, so I can also confirm that this works fine on linux, no troublesome linux bug still outstanding (or at least, I haven't run into it). I'm using mostly chrome(ium), but also tried it briefly on firefox and it worked. So if this was indeed due to some sort of linux issue, it seems to have gone by the wayside over the intervening years.
 
Veracity

Veracity

Developer
Staff member
Revision 11285.

Note that it did not show for me in Safari until I specifically told it to "delete website icons" and reload them. Boo caching that doesn't refresh when you simply refresh a page.
 
You must log in or register to reply here.
Top