Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

2017-03-03 Thread Kevin Rushforth



Mandy Chung wrote:

On Mar 3, 2017, at 10:29 PM, Kevin Rushforth  wrote:

Please review the following to update the FXML docs to document the requirement for a 
module that annotates non-public members with @FXML to "open" the containing 
package to the javafx.fxml module.

https://bugs.openjdk.java.net/browse/JDK-8170701
http://cr.openjdk.java.net/~kcr/8170701/webrev.00/



The doc change looks fine to me.

Minor suggestion.  You may refer "module-info class” to {@link 
ModuleDescriptor} which can be synthesized (although it may not be common).
  


I'll make this change.


You may consider referencing {@link Module#isOpened} where you may say “the 
containing package of the object is {@link Module#isOpened opened} to {@code 
javafx.fxml} module.
  


Good idea I will do that.

I'll post an updated webrev.

-- Kevin


Mandy


Re: [9] Review request: 8170702: Document that javafx.graphics needs explicit access to application main class

2017-03-03 Thread Kevin Rushforth



Mandy Chung wrote:

On Mar 3, 2017, at 10:36 PM, Kevin Rushforth  wrote:

[fixed subject line]

Please review the following to document that javafx.graphics needs explicit 
access to the Application class.

https://bugs.openjdk.java.net/browse/JDK-8170702
http://cr.openjdk.java.net/~kcr/8170702/webrev.00/



  69  * containing package must be {@link Module#isExported(String,Module) 
exported}

@linkplain instead?
  


I was following the pattern in Module.java, etc., which uses a regular 
@link in similar cases.



 239 StackTraceElement[] cause = Thread.currentThread().getStackTrace();

Good candidate to use StackWalker API.
  


This is pre-existing code (since JDK 7), and I don't want to change the 
implementation this late while fixing a doc bug. I will file a follow-on 
bug to consider improving this for JDK 10.



Is @throws RuntimeException an existing behavior?  I’d think CNFE and 
InaccessibleAE might be more appropriate.
  


Yes, this is the existing behavior and we are just documenting it. I 
agree that it might have been nicer to do something else, but the 
behavior w.r.t., exception is unchanged since JDK 8.



line 209 “It must be a subclass of Application or a RuntimeException will be 
thrown.”

I think this statement should be extended to cover if the class and its 
constructor are public and exported.
  


Yes, this seems like another good place to document the restriction. 
I'll post a .01 version of the webrev with this update.


-- Kevin



Mandy


[9] Review request: 8176167: Point to JDK 9 EA docs when building FX 9 docs

2017-03-03 Thread Kevin Rushforth

Dave,

Please review the following to point to the right place when we have a 
link to a JDK API when building the FX javadocs:


https://bugs.openjdk.java.net/browse/JDK-8176167

The one-line diff is in the bug report:

-- Kevin



Re: [9] Review request: 8170702: Document that javafx.graphics needs explicit access to application main class

2017-03-03 Thread Mandy Chung

> On Mar 3, 2017, at 10:36 PM, Kevin Rushforth  
> wrote:
> 
> [fixed subject line]
> 
> Please review the following to document that javafx.graphics needs explicit 
> access to the Application class.
> 
> https://bugs.openjdk.java.net/browse/JDK-8170702
> http://cr.openjdk.java.net/~kcr/8170702/webrev.00/

  69  * containing package must be {@link Module#isExported(String,Module) 
exported}

@linkplain instead?

 239 StackTraceElement[] cause = Thread.currentThread().getStackTrace();

Good candidate to use StackWalker API.

Is @throws RuntimeException an existing behavior?  I’d think CNFE and 
InaccessibleAE might be more appropriate.

line 209 “It must be a subclass of Application or a RuntimeException will be 
thrown.”

I think this statement should be extended to cover if the class and its 
constructor are public and exported.

Mandy

Re: [9] Review request: 8170701: Update FXML documentation for setAccessible

2017-03-03 Thread Mandy Chung

> On Mar 3, 2017, at 10:29 PM, Kevin Rushforth  
> wrote:
> 
> Please review the following to update the FXML docs to document the 
> requirement for a module that annotates non-public members with @FXML to 
> "open" the containing package to the javafx.fxml module.
> 
> https://bugs.openjdk.java.net/browse/JDK-8170701
> http://cr.openjdk.java.net/~kcr/8170701/webrev.00/

The doc change looks fine to me.

Minor suggestion.  You may refer "module-info class” to {@link 
ModuleDescriptor} which can be synthesized (although it may not be common).

You may consider referencing {@link Module#isOpened} where you may say “the 
containing package of the object is {@link Module#isOpened opened} to {@code 
javafx.fxml} module.

Mandy

[9] Review request: 8170702: Document that javafx.graphics needs explicit access to application main class

2017-03-03 Thread Kevin Rushforth

[fixed subject line]

Please review the following to document that javafx.graphics needs 
explicit access to the Application class.


https://bugs.openjdk.java.net/browse/JDK-8170702
http://cr.openjdk.java.net/~kcr/8170702/webrev.00/

-- Kevin


[9] Review request:

2017-03-03 Thread Kevin Rushforth
Please review the following to document that javafx.graphics needs 
explicit access to the Application class.


https://bugs.openjdk.java.net/browse/JDK-8170702
http://cr.openjdk.java.net/~kcr/8170702/webrev.00/

-- Kevin



[9] Review request: 8170701: Update FXML documentation for setAccessible

2017-03-03 Thread Kevin Rushforth
Please review the following to update the FXML docs to document the 
requirement for a module that annotates non-public members with @FXML to 
"open" the containing package to the javafx.fxml module.


https://bugs.openjdk.java.net/browse/JDK-8170701
http://cr.openjdk.java.net/~kcr/8170701/webrev.00/

-- Kevin



[9] Review request: JDK-8089548: [parfait] Memory leak in rt/modules/fxpackager/src/main/native/library/common/

2017-03-03 Thread Victor Drozdov

Chris,

Please review the changes related with parfait warnings. See my comments 
in JBS.


JIRA: https://bugs.openjdk.java.net/browse/JDK-8089548
Webrev: http://cr.openjdk.java.net/~vdrozdov/JDK-8089548/webrev.00/

--Victor



SwingNode black content ... still a problem

2017-03-03 Thread adam
https://bugs.openjdk.java.net/browse/JDK-8097328 is marked as
resolved...

However, using jdk-8u121-windows-i586 on Windows 7 I still see the
same issue. Black content in Swing Node until repaint is forced, for
example by dragging the window off-screen and on-screen again...
I also saw same issue with 8u40

The simplest case I come up with for this:

    public static void main(String[] args) {
    launch(args);
    }

    @Override
    public void start(Stage primaryStage) throws Exception {
    SwingNode swingNode = new SwingNode();
    SwingUtilities.invokeAndWait(() -> {
    swingNode.setContent(createSwingContent());
    });
    VBox vBox = new VBox(new Label("Fx content"), new
HBox(swingNode));
    primaryStage.setScene(new Scene(vBox));
    primaryStage.setWidth(300);
    primaryStage.setHeight(300);
    primaryStage.show();
    }

    private JComponent createSwingContent() {
    JPanel panel = new JPanel();
    panel.add(new JLabel("Swing content"));
    return panel;
    }