Griffon
  1. Griffon
  2. GRIFFON-614

Actions autocreated by ActionManager are being weird thread-wise

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.3.0
    • Component/s: rt
    • Labels:
    • Environment:
    • Number of attachments :
      1

      Description

      Actions created automatically by ActionManager must be handled in execInsideUIAsync blocks ie. enabling or disabling them has no effect if called directly inside the view or from a execSync block.

      Another possibly related symptom is that the "enabled" key in messages.properties has no effect on actions created automatically by ActionManager.

      Also, a JButton assigned to an automatically created action that gets its "name" from a key in messages.properties gets will get its preferred size's height set as if the button's caption was an empty string (too small, 12 pixels with nimbus, this is using the latest MigLayout plugin release).

        Activity

        Hide
        Andres Almiray added a comment -

        #1 value changes of action properties are automatically handled inside the UI thread (using async mode) by org.codehaus.griffon.runtime.swing.SwingGriffonControllerAction

        #2 fixed with https://github.com/griffon/griffon/commit/af6db618453796d6d2864c16eefa93cb6bc6bf85

        #3 could you please attach a minimal app that reproduces the problem?

        Show
        Andres Almiray added a comment - #1 value changes of action properties are automatically handled inside the UI thread (using async mode) by org.codehaus.griffon.runtime.swing.SwingGriffonControllerAction #2 fixed with https://github.com/griffon/griffon/commit/af6db618453796d6d2864c16eefa93cb6bc6bf85 #3 could you please attach a minimal app that reproduces the problem?
        Hide
        Edo added a comment -

        Hi,

        So #1 was fixed with #2 too?

        Attached a sample app that shows the problem behavior.

        Show
        Edo added a comment - Hi, So #1 was fixed with #2 too? Attached a sample app that shows the problem behavior.
        Hide
        Edo added a comment -

        Problem sample for #3

        Show
        Edo added a comment - Problem sample for #3
        Hide
        Andres Almiray added a comment -

        Yes, #1 and #2 go together. I can replicate #3 on OSX when changing L&F to either 'nimbus' or 'metal'. On OSX 'system' does the right thing, so I'm hesitant to put the blame on the Griffon side as 'system' should have the same effect as the others if this were a systemic bug, isn't it?

        Show
        Andres Almiray added a comment - Yes, #1 and #2 go together. I can replicate #3 on OSX when changing L&F to either 'nimbus' or 'metal'. On OSX 'system' does the right thing, so I'm hesitant to put the blame on the Griffon side as 'system' should have the same effect as the others if this were a systemic bug, isn't it?
        Hide
        Edo added a comment -

        Hmmm, if the default button height in the "system" L&F defaults to the equivalent of a blank space height it could be masking the issue. The fact that this only happens when letting ActionManager to set the button's text from messages.properties makes me feel like it's a timing issue...

        Actually I just did a quick test and it adds up, printing the "text" property for the buttons both before and after calling pack() in the dialog's controller shows it's null, but if you print it after app.windowManager.show(dialog) the button's text is set as expected. Calls to pack() would need to be intercepted to make sure that the widgets reflect the properties derived from their action configuration in messages.properties before the actual pack() is performed.

        Show
        Edo added a comment - Hmmm, if the default button height in the "system" L&F defaults to the equivalent of a blank space height it could be masking the issue. The fact that this only happens when letting ActionManager to set the button's text from messages.properties makes me feel like it's a timing issue... Actually I just did a quick test and it adds up, printing the "text" property for the buttons both before and after calling pack() in the dialog's controller shows it's null, but if you print it after app.windowManager.show(dialog) the button's text is set as expected. Calls to pack() would need to be intercepted to make sure that the widgets reflect the properties derived from their action configuration in messages.properties before the actual pack() is performed.
        Hide
        Andres Almiray added a comment -

        aha, your comment about a timing issue makes me think that setting action properties inside the EDT using invokeLater is the key to solve the problem. The framework instantiates a SwingGriffonControllerAction when a controller action is encountered. This class does is a wrapper around the real javax.swing.Action attached to builders. Every time a SGCA property changes value a PCE is triggered, handled by the j.s.Action with invokeLater. Thus, while the controller is being processed (inside a thread that is not the EDT) all j.s.Action properties will be set inside the EDT once there's a chance. Apparently that chance comes too late.

        Making sure SGCA properties are synced to the toolkit Action as soon as it's been configured should solve the problem. I'll update core, swing and javafx support to make it happen. Thanks for the thorough follow up.

        Show
        Andres Almiray added a comment - aha, your comment about a timing issue makes me think that setting action properties inside the EDT using invokeLater is the key to solve the problem. The framework instantiates a SwingGriffonControllerAction when a controller action is encountered. This class does is a wrapper around the real javax.swing.Action attached to builders. Every time a SGCA property changes value a PCE is triggered, handled by the j.s.Action with invokeLater. Thus, while the controller is being processed (inside a thread that is not the EDT) all j.s.Action properties will be set inside the EDT once there's a chance. Apparently that chance comes too late. Making sure SGCA properties are synced to the toolkit Action as soon as it's been configured should solve the problem. I'll update core, swing and javafx support to make it happen. Thanks for the thorough follow up.
        Hide
        Edo added a comment -

        Awesome! Thanks Andres.

        Show
        Edo added a comment - Awesome! Thanks Andres.
        Show
        Andres Almiray added a comment - Fixed with https://github.com/griffon/griffon/commit/8625eb9951ed44845edd58f2b9ef5e1a99a40fdf https://github.com/griffon/griffon-swing-plugin/commit/95aa5990bf7f3cedceed2788ffe44314439a6af8

          People

          • Assignee:
            Andres Almiray
            Reporter:
            Edo
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: