Hi Artem, On Fri, 2008-08-15 at 11:16 +0400, Artem Ananiev wrote: > Mark Wielaard wrote: > > Hi Artem, > > This looks like a different way to fix the issue I reported back in May > > for bug #6695441. I didn't see the review on the list and unfortunately > > the bug referenced #6708392 isn't available. So some questions about > > this solution.
BTW. This was my main point. The rest is a bit nitpicking the actual patch. If this is the solution that works for you then please do keep it. But it would be nice to have bugs like these public viewable and do reviews on the list so people have a bit more background about why a fix went in in the form it did. > > - The original used the solution of having the window name set to a > > special value. Although slightly hacky, this would in general work with > > any Toolkit that understood this hint. And it would prevent having to > > keep another cache. > > You have already answered the question: using Component's name to > specify an OverrideRedirect attribute is a hack. Do you think it is fine > to have such cross-toolkit hacks in our code? Maybe that hack was not ideal. But the point was that it was an attribute on the window itself. If you have an attribute on the window itself you can track these properties directly and don't need to invent any additional caches, which add overhead. > > - You add this to the generic SunToolkit class, but it seems specific to > > the XToolkit. It might be better to rename this property from > > OverrideRedirect to something X11/ICCCM specific. Maybe call it > > isPopupWindow? Then it is clear why some windows might have this hint > > set for their (swing) popups and then other toolkits/window/display > > managers could act appropriately. > > The answer for both questions is: java.awt.* classes should only expose > functionality available on all the supported platforms. Otherwise (like > in this case with OverrideRedirect hint), sun.awt.* packages are used. > If we later find a Win32 hint which corresponds to X11 OverrideRedirect, > we'll move the code from sun.awt.* to java.awt.* and/or make it public. > > I understand the current solution is not ideal, however it is still > better than having all the windows OverrideRedirect or a new method in > Window class. The main reason I asked is because I saw the Caciocavallo project go through great lengths to make AWT toolkits easier portable. http://openjdk.java.net/projects/caciocavallo/ I believe the functionality that needs exposing is the attribute of an window being a pupup menu/window. The OverrideRedirect is too narrow in my view and to specific to X11/ICCCM. So indeed I would have expected to have some interface to mark a Window(Peer) as a popup instead of something specific to X11 like override redirect. This solution is certainly better than having all the X11 windows set OverrideRedirect (although that was a solution for a different problem #6669302 Window.toFront()/toBack() not working with metacity). I do think it would not be bad to have Window or WindowPeer have an extra method/field to mark it as a popup window though. Cheers, Mark
