Bug 105982 - Adding favicon.ico to the garden-o-matic.
Summary: Adding favicon.ico to the garden-o-matic.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim 'mithro' Ansell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-02 18:41 PST by Tim 'mithro' Ansell
Modified: 2013-01-03 10:07 PST (History)
5 users (show)

See Also:

Attachments
Patch (2.72 KB, patch)
2013-01-02 18:42 PST, Tim 'mithro' Ansell 		no flags Details | Formatted Diff | Diff
Screenshot of the favicon.ico working (4.77 KB, image/png)
2013-01-02 18:45 PST, Tim 'mithro' Ansell 		no flags Details
Patch (2.72 KB, patch)
2013-01-02 20:16 PST, Tim 'mithro' Ansell 		no flags Details | Formatted Diff | Diff
Patch (2.43 KB, patch)
2013-01-02 21:30 PST, Tim 'mithro' Ansell 		no flags Details | Formatted Diff | Diff
Patch (2.14 KB, patch)
2013-01-02 22:05 PST, Tim 'mithro' Ansell 		no flags Details | Formatted Diff | Diff
Patch (2.26 KB, patch)
2013-01-03 01:01 PST, Tim 'mithro' Ansell 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tim 'mithro' Ansell 2013-01-02 18:41:43 PST 
Adding favicon.ico to the garden-o-matic.
Comment 1 Tim 'mithro' Ansell 2013-01-02 18:42:55 PST 
Created attachment 181130 [details]
Patch
Comment 2 Tim 'mithro' Ansell 2013-01-02 18:45:17 PST 
Created attachment 181132 [details]
Screenshot of the favicon.ico working
Comment 3 WebKit Review Bot 2013-01-02 19:31:05 PST 
Comment on attachment 181130 [details]
Patch

Attachment 181130 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15628766

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Comment 4 Tim 'mithro' Ansell 2013-01-02 20:16:57 PST 
Created attachment 181140 [details]
Patch
Comment 5 Adam Barth 2013-01-02 20:35:05 PST 
Why not just use a real file?
Comment 6 Tim 'mithro' Ansell 2013-01-02 21:02:38 PST 
I guess I could use a real file, if you can give me the location to put the file (that is gardener specific)?
Comment 7 Adam Barth 2013-01-02 21:07:21 PST 
Take a look at where the party time graphic is stored.
Comment 8 Tim 'mithro' Ansell 2013-01-02 21:30:06 PST 
Created attachment 181145 [details]
Patch
Comment 9 Tim 'mithro' Ansell 2013-01-02 21:33:02 PST 
Moved to using an on-disk file.
Comment 10 Adam Barth 2013-01-02 21:52:32 PST 
Comment on attachment 181145 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181145&action=review

> Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:34
> +import base64
>  import logging
>  import json
>  import os
>  import sys
> +import tempfile
>  import urllib
> +import zlib

These imports are no longer needed.

> Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:123
> +        self._serve_file('garden-o-matic.ico')

Why not call it favicon.ico or have the web app use the name garden-o-matic.ico.  It seems odd to change the name here.

In fact, you don't need code at all.  You can just add "ico" to STATIC_FILE_EXTENSIONS
Comment 11 Tim 'mithro' Ansell 2013-01-02 22:05:06 PST 
Created attachment 181150 [details]
Patch
Comment 12 Tim 'mithro' Ansell 2013-01-02 22:06:13 PST 
Comment on attachment 181145 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181145&action=review

>> Tools/Scripts/webkitpy/tool/servers/gardeningserver.py:123
>> +        self._serve_file('garden-o-matic.ico')
> 
> Why not call it favicon.ico or have the web app use the name garden-o-matic.ico.  It seems odd to change the name here.
> 
> In fact, you don't need code at all.  You can just add "ico" to STATIC_FILE_EXTENSIONS

I was worried that would make this icon used for everything, not just the garden-o-matic.
Comment 13 Adam Barth 2013-01-02 23:29:52 PST 
Comment on attachment 181150 [details]
Patch

It won't.
Comment 14 Tim 'mithro' Ansell 2013-01-03 01:01:49 PST 
Created attachment 181155 [details]
Patch
Comment 15 Tim 'mithro' Ansell 2013-01-03 01:06:23 PST 
As requested. 

The other option was to put a meta tag like below;
<link rel="icon" href="http://www.example.com/garden-o-matric.ico" type="image/x-icon">
Comment 16 Adam Barth 2013-01-03 09:42:00 PST 
Comment on attachment 181155 [details]
Patch

Thanks for iterating on the patch.
Comment 17 WebKit Review Bot 2013-01-03 10:06:57 PST 
Comment on attachment 181155 [details]
Patch

Clearing flags on attachment: 181155

Committed r138713: <http://trac.webkit.org/changeset/138713>
Comment 18 WebKit Review Bot 2013-01-03 10:07:02 PST 
All reviewed patches have been landed.  Closing bug.