[rfc][icedtea-web] bringing applets out of browser (part1)

Thu Dec 18 17:04:15 UTC 2014

>
> I meant the downloading part which I guess is what you wanted. Can you add to the comments of the function that if the file isn't in cache, it will block until downloaded and then return.

fixed

>
>>>
>>> Actually this is the only *correct*  way, how any resource should be
>>> downloaded. So the method
>>> should be public, and actually highlighted to prevent others from inviting
>>> wheel.
>>>
>>> :( M imagination failed and stayed with this as with best possible name.
>>> It is Return from AccessWarningPane, and is Complex(not plain number) So I
>>> really think
>>> AccessWarningPaneComplexReturn is good. If you strongly disagree, you will
>>> have to invent one :P
>>>
>>>>
>>>> Maybe we should consider replacing AccessWarningPane with this class
>>>> slightly expanded?
>>>
>>> What do you mean?
>
> I meant: Now that we have something that can handle complex returns, can't it also handle the old integer returns as well? Why do we need both AccessWarningPane and AccessWarningPaneComplexReturn?
>
Sorry, Is till do not understand.
AccessWarningPane IS returning AccessWarningPaneComplexReturn

How can we not need both?
>
> Definitely not for this patch though.

good :)
>
>
>>>
>>>>
>>>> [...]
...
>>>
>>> I would like to have the tooltip to complete the buttons description. so my
>>> sentences:
>>>
>>> +EXAWrememberByAppTooltip=<html>This application will never ask more
>>> permissions questions</html>
>
> s/ask more/ask for more

Fixed. I hope the sentences give you sense. I was manytimes criticized for my english :(
>
>>>
>>> It must be clear, that  that *all* permissions are approved/declined by
>>> this *forever*, unless they
>>> are changed in itweb-settings
>>>
>>> +EXAWrememberByPageTooltip=<html>All applications from this domain will
>>> stop asking, and will follow
>>> your current decision on all permissions</html>
>>>
>>>
>>> Sae, with sound on *domain*
>>>
>>> Both above are *not* *yet* implemented But I would like to do so for 1.6
>>> The following one is
>>> implemented (== no change in current behaviour)
>>>
>>> +EXAWdontRememberTooltip=<html>Your decision will be used only for this
>>> single permission for this
>>> single run</html>
>>>
>>> It must be clear that it will keep asking for all other types of
>>> permissions.
>>>
>>> So maybe add "use in this runtime for all permissions" is probably missing
>>> option :)
>
> Yeah. If you'd like you can add this in another patch later.
>

It is on top of my todo for this menu/html support.
>>>
>>>
>>>
...
>>> in other security.Dialogs. One is using this to returnn, one something
>>> else, some supports
>>> remmbering, some do not....
>>>
>>>   From me its +1 for object and lets every dialog returns what it wants.
>>>
>>> If those should be unified, it will be a lot of work.
>
> -        Integer buttonIndex;
> +        Object buttonIndex;
>           SecurityDialog dialog;
>
>           public SetValueHandler(SecurityDialog dialog, int buttonIndex) {
>
> The change to Object is pretty useless if the single constructor for the class accepts an integer. You should change the 'int buttonIndex' to Object.
>
> At the same time, looking at the code, there is only one use of the constructor:
>
>      protected ActionListener createSetValueListener(SecurityDialog dialog, int buttonIndex) {
>          return new SetValueHandler(dialog, buttonIndex);
>      }
>
> This only accepts integers as well. So you'd need to change this to Object as well.
>
>
> Otherwise in the end, it always accepts integers, and you should NOT change Integer buttonIndex to Object buttonIndex.
> Looking at your changes, you just override this listener anyways.
>
> +        //override the stupid createSetValueListener mechanism
> +        run.addActionListener(new ActionListener() {
> +
> +            @Override
> +            public void actionPerformed(ActionEvent e) {
> +                parent.setValue(getModifier(0)); //according to  createSetValueListener 0 is ok and 1 cancel
> +                parent.dispose();
> +            }
> +        });
>
> It seems you don't even want to use the SetValueHandler at all. Might as well remove it's use where you don't need it.
>
> Reading over your changeset, I am nearly 100% sure this change isn't needed.
>
>
>
>
> Having Objects will eventually require users to expect Objects as parameters and here's a discussion-thread on why Objects should be avoided as parameters: http://programmers.stackexchange.com/questions/198085/is-it-poor-programming-practice-to-pass-parameters-as-objects.
>
>
> Fixing at this point would probably require a lot of work, so up to you.

Although I moreover agree with your statements, The object as parameter != object as return value or 
keeping value. I think usage of this patch is quite well.

On oposite - I don't like my solution.  When I willbe hacking on rembering the action, I will 
probably elaborate around this  a lot.
>
>
>
>>>>
>>>> I think using Object directly is not very safe. How about making a new

>>>
>>> I'm not sure if I wont to change it... Maybe yes... Well.. how? Some
>>> joptionpane? Where? hmhmh...
>>> not sure baout this...
>>>
>>> IF the run from shortcut fails, then it is properly reported.
>>>
>
> It's fine if there are logs that report failure of generation. It would be great if the user could know whether or nut it was successfully created without checking logs. Just throw up a message dialog that tells them whether or not it worked. Allowing users to attempt to create shortcuts, and not telling them if it worked or not seems really poor.

I agree. Again, on top of my "known bugs" list for those menu/html support.
>
>
> AS well, a few tiny nits in-line below:
>
>
all fixed.
>       }
> +
> +
> +    public String createJnlpVendorValue() {
>
> I think there is an extra line here
>
>
>
> +        //override the stupid createSetValueListener mechanism
> +        run.addActionListener(new ActionListener() {
>
> I'd remove the word 'stupid', it seems very unprofessional
>
>
> +
> +        private void setTolltips() {
> +            browser.setToolTipText(R("EXAWbrowserTolltip"));
>
> s/Toll/Tool
>
>
> +    //note this time is in ESST timezone, and so is expecting the strring ouutput below
> s/ESST/EST/  s/strring/string/  s/ouutput/output/
>
>
>
>
>
>
ok to go?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: appletsOutOfBrowserShortcuts-head-05.patch
Type: text/x-patch
Size: 91214 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20141218/615ba21a/appletsOutOfBrowserShortcuts-head-05-0001.patch>