[7u6] request for review: 7124328: [macosx] javax.swing.JDesktopPane.getAllFramesInLayer returns unexpected value (original) (raw)
Anthony Petrov anthony.petrov at oracle.com
Tue Apr 17 06:34:21 PDT 2012
- Previous message: [7u6] request for review: 7124328: [macosx] javax.swing.JDesktopPane.getAllFramesInLayer returns unexpected value
- Next message: [7u6] request for review: 7124328: [macosx] javax.swing.JDesktopPane.getAllFramesInLayer returns unexpected value
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
OK then. The fix is fine anyway.
-- best regards, Anthony
On 4/17/2012 5:30 PM, Alexander Potochkin wrote:
Hello Anthony
Hi Alex,
The fix looks good to me. thanks Though if I were you, I would add an argument to the new static method to allow for inclusion of internal frame from a specified layer only, or all layers if the argument is equal to e.g. MAXINT. This way we could avoid adding and then removing some of the found frames to/from the collection. MAXINT is a valid parameter for JLayeredPane.putLayer(JComponent, int), negative values are also ok, for example JLayeredPane.FRAMECONTENTLAYER is defined as -30000 so there is no "reserved" integer to serve as a special flag for frames in all layers alexp
-- best regards, Anthony On 4/16/2012 10:30 PM, Alexander Potochkin wrote: Hello Anthony
Thanks for reviewing it, here is the new version: http://cr.openjdk.java.net/~alexp/7124328/webrev.01/
Hi Alex,
It seems like the new private getAllFrames(Contianer) method may be declared as static. Also I suggest to insert spaces between "if"s and "("s at lines 275, 277, and 282. Otherwise the changes look good. fixed
BTW, do we have any regression tests for this method? Do they pass? It's a pretty good question :-) There is no regression tests for JDesktopPane.getAllFrames() but I've found a regression test for the old bug 4132993 which fixes the problem with JDesktopPane.getAllFramesInLayer(). This method also should return iconified frames in a specified layer, it didn't work for Aqua so I fixed it as well What about the performance regression caused by iterating over the inner containers? No observable performance regression. Thanks alexp -- best regards, Anthony On 4/10/2012 9:25 PM, Alexander Potochkin wrote: Hello Please review the following fix: http://cr.openjdk.java.net/~alexp/7124328/webrev.00/ for this bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7124328 the spec for the method javax.swing.JDesktopPane.getAllFrames() says * Returns all
JInternalFrames
currently displayed in the * desktop. Returns iconified frames as well as expanded frames. but the AquaLookAndFeel on MacOS uses a special container for the minimized internal frames and that's why the current implementation doesn't see them the proposed fix recursively looks for the internal frames among the JDesktopPane's children the test can be found here: http://java.net/jira/browse/MACOSXPORT-557 Thanks alexp
- Previous message: [7u6] request for review: 7124328: [macosx] javax.swing.JDesktopPane.getAllFramesInLayer returns unexpected value
- Next message: [7u6] request for review: 7124328: [macosx] javax.swing.JDesktopPane.getAllFramesInLayer returns unexpected value
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]