[rfc][icedtea-web] manager for desktop integration

[Jiri Vanek]

On 10/26/2015 06:45 PM, Jacob Wisor wrote:
> On 10/26/2015 at 03:39 PM Jiri Vanek wrote:
>> On 10/23/2015 05:46 PM, Jacob Wisor wrote:
>>> On 10/23/2015 at 04:45 PM Jiri Vanek wrote:
>>>> Hello.
>>>>
>>>> here is long ago promised manager for desktop integration.
>>>>
>>>> Considering how compact the change is, I'm playing with idea of 1.6 backport
>>>> (who knows when 1.7 will go alive....)
>>>>
>>>> The ico class is from this article:
>>>> http://www.informit.com/articles/article.aspx?p=1186882&seqNum=2
>>>> I contacted the author, and got no response in last two days. I did nto found
>>>> any license...
>>>> If he will not reply - drop the ico-format view? Depend on image4j?
>>>
>>> Hmm, could you please explain why IcedTea-Web should need an ico file decoder?
>>> AFAIK, the JNLP
>>
>> Reasonable question.
>>
>> When  generates files for desktop integration, when all other ways fails, it
>> tries also favicon.ico
>> Generally guessed, there can be quite a couple of those in cons' cache dir.
>
> Yeah, it is probably a good idea to get hold of the favicon if no particular icon has been provided
> by the software vendor or everything else fails. However, you get to choose only one favicon, if
> any, for possibly multiple applications on one HTML page. What do you do then? How do make those
> apps distinguishable? Then surely only by name.

Of course by name. So I guess you did not bother to deploy the patch.

>
> Btw, I hope you do not mean IcedTea-Web should, for lack of a better icon, just start fishing for
> any arbitrary icon in the cache directory. Do you?

Of course not. I guess you did not bothered to check how desktop integration is working.

If even favicon fails, javaws icon is used.
>
>> In this patch, when list of all cached icons is populated, previews are
>> generated. It would be sad to miss preview of .ico files (as short-cuts visual
>> icon is for desktop usage most reminding thing)
>>
>> So generally not needed, but nice to have.
>
> Indeed, it would be nice to have. But, I am not convinced we should deploy an image decoder with
> IcedTea-Web. Sounds like an overkill to me, even if it has a small storage and memory footprint.

The parser of ico is absolutely simple thing.
Arr you speaking about memory foot print of bitmap of  few x little bit more px? What it have to do 
with this?

No metter if I use custom parser or javaIO.read at the end is buffered image. And javaIO very often 
uses native parsers... So what you wont me here to do?

> Nevertheless, for the lack of better support from the Java class library, I would accept adding an
> ico image decoder to IcedTea-Web if
> 1. it is implemented via javax.imageio.spi and

[*]

> 2. its storage and memory footprint is as small as possible.

The logic is veryu small java file. The memory print will be be same always - at the end it is 
matrix of pixels Nothign more, nothing less.

Not sure where you are pointing by this.
>
> I did not take a closer look at the implementation you suggested but it does not seem to support
> javax.imageio.spi. If this favicon feature is really important to you I would suggest you search for
> an ico image decoder implementation supporting javax.imageio.spi. Or, adapt the existing to
> javax.imageio.spi. Maybe the author will show up eventually?

[**]
>
>>> specification only allows internet image file formats, like png, jpg, gif, and
>>> mng, but does not
>>> accept ico files for applet/application icons.
>
> Hmm, I have just looked it up at IANA. The ico file format did get indeed registered as
> image/vnd.microsoft.icon in 2003. Who would have thought? However, implementing an image file format
> decoder for a standard sounds more like a job for the Java class library than an application, even
> like an implementation of JNLP.

Well.. I did not know this either. And it is good to know.
 From this point have [*] muchj more sense.
As for submitting this upstream to java RT.. ouch. It would be painfull... I'm not going ot do so. 
Lets somebody else torture by this.


>
>>>> I'm for inclusion, and removal in case of complain.
>>>
>>> No, let's not haste again, okay? Please, respect the license and thus respect
>>> the order of doing
>>
>> If there would be some license to respect... :( But otherwise ok. He did not
>> write up to now, so I guess ico support is going away unless you agree on
>> including Ico.java and possible delete on Authors complain)
>
> No, we should not just take any code off the internet and redistribute it with a new name and
I kept old name. Only used jnlp pakage.

> license on it. Currently, the license is bogus at best. Slapping code together from random sites is

hmhmh. That sounds right. I intentionally used  original netx license.  I'm ok with no (original) 
license on that file
> what script kiddies do, not professionals. Just because the author is out of reach now and the

he may be out for ever.
> license is bogus, we should not start assuming that nobody cares and eventually nobody will notice.
> This is generally a very bad assumption to make.

I really dont know what is lesser evil. To skip ICO support, to write own reader, to
>
>> (Ico is moreover wrapper about png anyway.
>
Ok. My wrong in words. ico CAN be wraper about png (funny isnt it :) )

> Err, no. ico is *nothing* like png. :-D Where did you get this?

Luckily, you are wrong.
https://en.wikipedia.org/wiki/ICO_%28file_format%29#PNG_format

>
>> If we disagree here to much, I will write my own very probably...)
>
> So, it does seem important to you.

yes, otherwise I would not bother.
> Well, then I think you got yourself something to do. :-)
> Anyways, a few comments on the patch:
>
> Please try to replace most of the code where you concatenate strings with StringBuilder.append() (or
> StringBuilder.xxx() etc) where applicable, but especially where you build some HTML for preview.
> Please also remember to chain StringBuilder calls much as possible. This should give best
> performance results. Although the compiler does some "automagic" StringBuilder conversion when using
> the + operator on strings, it still lacks the analysis quality of a human brain and thus does not do
> much chaining. ;-)

Sure .. will be done.


>
> Another thing that I have noticed is that there are some places where you seem to be doing quite a
> lot of stuff on the AWT thread, which may lead to blocking it for too long. This may get even worse
> if you get a lot files or input data on the target machine to handle. So, it may be a good idea to
> move all this analyzing, generating, and decoding stuff in to a worker thread.

There absolutely no IO operations which may possibly take so long that awt queue can freze.
>
> Reagrds,
>
> Jacob


[*]
in light of [**]..... I will go with the patch without ico support.
And will again[***] try spi provider. As it may be really usefull. And post ico support as separate 
patch. Lets see how it will be sucessful as.. bad luck... I already[***] tried to add wraper about 
this Ico.java providing spi support. After several hours I discarded it as not necessary. But 
because of [**]  lets try again....


J.