[rfc][icedtea-web][policyeditor] Fix for duplicate codebases when launched from dialog

Jiri Vanek jvanek at redhat.com
Wed Jun 18 10:11:30 UTC 2014


On 06/17/2014 10:30 PM, Andrew Azores wrote:
> On 06/17/2014 04:08 PM, Jie Kang wrote:
>> Hello,
>>
>>> Hi,
>>>
>>> When launching PolicyEditor from a security dialog, the applet's
>>> codebase may be added to the UI twice. This only affects the list UI -
>>> each entry shares the same checkbox state, custom permissions, etc., and
>>> will be written to file as one entry. This happens partially because
>>> there was some missing validation that the element doesn't already exist
>>> before it is re-added to the list, and partially because in another
>>> place where validation was done, the check was performed outside of a
>>> SwingUtilities.invokeLater block which would later update the UI with
>>> the new element, without re-checking its presence. This could cause a
>>> race condition where the element was not yet in the list when it was
>>> checked but could have already been added by the time it the invokeLater
>>> ran. This patch fixes these two errors.
>>>
>>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1850
>>>
>>
>> Patch looks good. 0.5.
>>
>> Maybe add javadoc that duplicate codebase urls aren't accepted?
>>
>> Also I just noticed but there is javadoc in PolicyEditor on the weakReference use. Not sure what
>> the accepted style is in regards to comments vs. Javadoc but I think implementation details like
>> why the weakReference is used would be more appropriate as a comment rather than part of Javadoc.
>>
>
> Ehh, being pedantic, but Javadoc *are* just comments. It's just that your IDE and potentially the
> javadoc tool, if you use it, decide to handle specific comments in a special manner ;). I can remove
> the one extra asterisk that makes it trigger the special handling, but I don't think it's really a
> big deal stylistically. None of it is public API anyway. The WeakReference is fairly questionable on
> why it's needed anyway so for people using an IDE, having it as javadoc allows that extra note on it
> to be visible just by eg hovering the cursor over any appearance of the identifier, not just by
> jumping to view the definition. So I'm in favour of just leaving it as-is.
>
+1 for keeping it as it is.

My personal feelingsare for more javadocs then comments.

But usage of brain when to you which (javadco x just comment) is sometimes extraordinary hard, so 
dont take me like magic bullet :)
J.



More information about the distro-pkg-dev mailing list