[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-awt-dev
Subject:    Re: <AWT Dev> Review-request for 8143227: Platform-Specific Desktop Features
From:       Alexander Zvegintsev <alexander.zvegintsev () oracle ! com>
Date:       2015-11-24 15:02:31
Message-ID: 56547C07.7090502 () oracle ! com
[Download RAW message or body]

Please review the updated fix:
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/

removed fullscreen related code (moved to JDK-8143914 [0])
fix serialization in AppEvent

[0] https://bugs.openjdk.java.net/browse/JDK-8143914

Thanks,

Alexander.

On 11/21/2015 03:33 AM, Alexander Zvegintsev wrote:
> Hi Phil,
>
>> Can someone explain why this is needed given the existing support of
>> GraphicsDevice.setFullScreenWindow(Window) ? 
>
> GraphicsDevice.setFullScreenWindow is used for an exclusive full 
> screen mode.
> Mac OS has another option to create a virtual desktop with provided 
> window in it.
> You can switch between them by three-finger horizontal swipe.
>
>> Why does it have to be a RootPaneContainer ? Why is this tied to Swing ?
>> This appears to narrow it to JDialog and JWindow. 
> It reuses swing code to set native windows style bits.
>
>
> Please see updated webrev:
> http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/
> updated permission
> added missing @throws @since and @implNote
> browseFileDirectory is now return void
> RootPaneContainer -> JDialog and JWindow
>
> -- 
> Thanks,
> Alexander.
>
> On 11/20/2015 09:03 PM, Phil Race wrote:
>> On 11/20/2015 09:12 AM, Sergey Bylokhov wrote:
>>>
>>> I am worried about setWindowCanFullScreen and requestToggleFullScreen.
>>> On the latest osx this functionality was merged with maximize 
>>> button. So probably it will be better to change behavior of 
>>> window.setExtendedState() + MAXIMIZED_BOTH?
>>
>> Can someone explain why this is needed given the existing support of
>> GraphicsDevice.setFullScreenWindow(Window) ?
>>
>> And what happens if you use *both* ? They still need to play well 
>> together
>> if there is some reason the new one is needed.
>>
>> Other comments :
>> >   * Note, Aqua Look and Feel should be active to support this on 
>> Mac OS.
>>
>>
>> Needs @implNote
>>
>> There seems to be lots of missing SecurityException tags given all 
>> the checkAWTPermission() calls.
>> is checkAWTPermission() really the right call for all of these actions ?
>> Does it "cover" being able to delete files and quit the app ? I am 
>> not sure it is
>> correct in all cases.
>>
>> And also there are missing @since tags.
>>
>>
>>  Opens a folder containing the {@code file} in a default system file 
>> manager.
>>  933      * @param file the file
>>  934      * @return returns true if successfully opened
>>  935      * @throws NullPointerException if {@code file} is {@code null}
>>  936      * @throws IllegalArgumentException if the specified file 
>> doesn't
>>  937      * exist
>>  938      */
>>  939     public boolean browseFileDirectory(File file) {
>>
>> So what happens if there is no "support" for this ? Exception or 
>> "false" ?
>> Are you comfortable that all these APIs that return "true" if 
>> successful are
>> implementable on all platforms. i.e I mean that does the platform return
>> a value you can pass on as success/failure.
>>
>> ---
>>
>> 861      * Attaches a {@link FullScreenListener} to the specified 
>> top-level
>>  862      * {@link Window}.
>>  863      *
>>  864      * @param window to attach the {@link FullScreenListener} to
>>  865      * @param listener to be notified when a full screen event 
>> occurs
>>  866      * @throws IllegalArgumentException if window is not a
>>  867      * {@link javax.swing.RootPaneContainer}
>>  868      */
>>  869     public void addWindowFullScreenListener(final Window window,
>>  870                                               final 
>> FullScreenListener listener) {
>>
>> -------
>>
>> Why does it have to be a RootPaneContainer ? Why is this tied to Swing ?
>> This appears to narrow it to JDialog and JWindow.
>>
>> -phil.
>>
>


[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Please review the updated fix:<br>
    <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/">http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/03/</a><br>
  <br>
    removed fullscreen related code (moved to JDK-8143914 [0])<br>
    fix serialization in AppEvent
    <meta http-equiv="content-type" content="text/html; charset=utf-8">
    <br>
    <br>
    [0] <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8143914">https://bugs.openjdk.java.net/browse/JDK-8143914</a><br>
  <br>
    <pre class="moz-signature" cols="72">Thanks,

Alexander.</pre>
    <div class="moz-cite-prefix">On 11/21/2015 03:33 AM, Alexander
      Zvegintsev wrote:<br>
    </div>
    <blockquote cite="mid:564FBBD3.1040208@oracle.com" type="cite">Hi
      Phil,
      <br>
      <br>
      <blockquote type="cite">Can someone explain why this is needed
        given the existing support of
        <br>
        GraphicsDevice.setFullScreenWindow(Window) ? </blockquote>
      <br>
      GraphicsDevice.setFullScreenWindow is used for an exclusive full
      screen mode.
      <br>
      Mac OS has another option to create a virtual desktop with
      provided window in it.
      <br>
      You can switch between them by three-finger horizontal swipe.
      <br>
      <br>
      <blockquote type="cite">Why does it have to be a RootPaneContainer
        ? Why is this tied to Swing ?
        <br>
        This appears to narrow it to JDialog and JWindow. </blockquote>
      It reuses swing code to set native windows style bits.
      <br>
      <br>
      <br>
      Please see updated webrev:
      <br>
      <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/">http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/02/</a>
  <br>
      updated permission
      <br>
      added missing @throws @since and @implNote
      <br>
      browseFileDirectory is now return void
      <br>
      RootPaneContainer -&gt; JDialog and JWindow
      <br>
      <br>
      --
      <br>
      Thanks,
      <br>
      Alexander.
      <br>
      <br>
      On 11/20/2015 09:03 PM, Phil Race wrote:
      <br>
      <blockquote type="cite">On 11/20/2015 09:12 AM, Sergey Bylokhov
        wrote:
        <br>
        <blockquote type="cite">
          <br>
          I am worried about setWindowCanFullScreen and
          requestToggleFullScreen.
          <br>
          On the latest osx this functionality was merged with maximize
          button. So probably it will be better to change behavior of
          window.setExtendedState() + MAXIMIZED_BOTH?
          <br>
        </blockquote>
        <br>
        Can someone explain why this is needed given the existing
        support of
        <br>
        GraphicsDevice.setFullScreenWindow(Window) ?
        <br>
        <br>
        And what happens if you use *both* ? They still need to play
        well together
        <br>
        if there is some reason the new one is needed.
        <br>
        <br>
        Other comments :
        <br>
        &gt;     * Note, Aqua Look and Feel should be active to support
        this on Mac OS.
        <br>
        <br>
        <br>
        Needs @implNote
        <br>
        <br>
        There seems to be lots of missing SecurityException tags given
        all the checkAWTPermission() calls.
        <br>
        is checkAWTPermission() really the right call for all of these
        actions ?
        <br>
        Does it "cover" being able to delete files and quit the app ? I
        am not sure it is
        <br>
        correct in all cases.
        <br>
        <br>
        And also there are missing @since tags.
        <br>
        <br>
        <br>
          Opens a folder containing the {@code file} in a default system
        file manager.
        <br>
          933           * @param file the file
        <br>
          934           * @return returns true if successfully opened
        <br>
          935           * @throws NullPointerException if {@code file} is
        {@code null}
        <br>
          936           * @throws IllegalArgumentException if the specified
        file doesn't
        <br>
          937           * exist
        <br>
          938           */
        <br>
          939         public boolean browseFileDirectory(File file) {
        <br>
        <br>
        So what happens if there is no "support" for this ? Exception or
        "false" ?
        <br>
        Are you comfortable that all these APIs that return "true" if
        successful are
        <br>
        implementable on all platforms. i.e I mean that does the
        platform return
        <br>
        a value you can pass on as success/failure.
        <br>
        <br>
        ---
        <br>
        <br>
        861           * Attaches a {@link FullScreenListener} to the
        specified top-level
        <br>
          862           * {@link Window}.
        <br>
          863           *
        <br>
          864           * @param window to attach the {@link
        FullScreenListener} to
        <br>
          865           * @param listener to be notified when a full screen
        event occurs
        <br>
          866           * @throws IllegalArgumentException if window is not a
        <br>
          867           * {@link javax.swing.RootPaneContainer}
        <br>
          868           */
        <br>
          869         public void addWindowFullScreenListener(final Window
        window,
        <br>
          870                                                                         \
final  FullScreenListener listener) {
        <br>
        <br>
        -------
        <br>
        <br>
        Why does it have to be a RootPaneContainer ? Why is this tied to
        Swing ?
        <br>
        This appears to narrow it to JDialog and JWindow.
        <br>
        <br>
        -phil.
        <br>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic