[rfc][icedtea-web] bringing applets out of browser (part1)
Jie Kang
jkang at redhat.com
Thu Dec 18 18:14:04 UTC 2014
----- Original Message -----
> >
> > 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?
I was under the impression that AccessWarningPaneComplexReturn was a pane, but it's actually a return object. I see now that I misunderstood.
Okay. I will try to think of a name, but this is fine now.
> >
> > 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.
> > - Integer buttonIndex;
> > + Object buttonIndex;
This code is inside SetValueHandler.java:
This class has 1 constructor:
public SetValueHandler(SecurityDialog dialog, int buttonIndex) {
This sets:
this.buttonIndex = buttonIndex : Notice how constructor has 'int' : Nobody can give it anything but an integer
This constructor is used in 1 place:
protected ActionListener createSetValueListener(SecurityDialog dialog, int buttonIndex) {
return new SetValueHandler(dialog, buttonIndex);
}
Notice how here as well: It only accepts integer for buttonIndex.
Nothing can give it an Object at the moment. Keeping it as Integer is correct, unless you make changes so people can give it an Object.
Can you point out where exactly this change to Object is used? I can understand your argument but, when I read the code, it seems it doesn't ever get used.
The above is my only concern.
Regards,
> >
> >
> >
> >>>>
> >>>> 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?
>
>
--
Jie Kang
More information about the distro-pkg-dev
mailing list