Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reflection error trying to set private field to a background color (custom JPanel) #110

Closed
Sciss opened this issue Apr 8, 2014 · 14 comments
Assignees

Comments

@Sciss
Copy link
Contributor

Sciss commented Apr 8, 2014

It seems that WebLaF is using heavy reflection to "poke" into private fields of components. I don't know exactly why it's doing that but it runs into a problem with DockingFrames:

java.lang.IllegalArgumentException: Can not set bibliothek.gui.dock.util.BackgroundAlgorithm field bibliothek.gui.dock.util.BackgroundPanel.background to java.awt.Color
    at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:164)
    at sun.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:168)
    at sun.reflect.UnsafeObjectFieldAccessorImpl.set(UnsafeObjectFieldAccessorImpl.java:81)
    at java.lang.reflect.Field.set(Field.java:741)
    at com.alee.managers.style.skin.WebLafSkin.setFieldValue(WebLafSkin.java:477)
    at com.alee.managers.style.skin.WebLafSkin.applyProperties(WebLafSkin.java:434)
    at com.alee.managers.style.skin.WebLafSkin.applySkin(WebLafSkin.java:257)
    at com.alee.managers.style.skin.WebLafSkin.applySkin(WebLafSkin.java:192)
    at com.alee.managers.style.StyleManager.applySkin(StyleManager.java:310)
    at com.alee.managers.style.StyleManager.applySkin(StyleManager.java:290)
    at com.alee.laf.panel.WebPanelUI.installUI(WebPanelUI.java:106)
    at javax.swing.JComponent.setUI(JComponent.java:655)
    at javax.swing.JPanel.setUI(JPanel.java:153)
    at javax.swing.JPanel.updateUI(JPanel.java:126)
    at javax.swing.JPanel.<init>(JPanel.java:86)
    at javax.swing.JPanel.<init>(JPanel.java:109)
    at javax.swing.JPanel.<init>(JPanel.java:117)
    at bibliothek.gui.dock.util.BackgroundPanel.<init>(BackgroundPanel.java:57)

It seems that WebPanelUI assumes there is a field background which is of type java.awt.Color. This is a private field in JComponent perhaps, but the component in question here, has the following:

public abstract class BackgroundPanel extends JPanel implements PaintableComponent{
    private BackgroundAlgorithm background;
    ...

So this throws a reflection error. It is caught, but means that the console is full of stack traces. Somehow the reflection check against the field type must go wrong.

@mgarin
Copy link
Owner

mgarin commented Apr 21, 2014

Sorry for the delay, was away for two weeks.

Yes, new styling system uses reflection a lot - it basically sets all values from the style XML using reflection and calling either a setter method or directly modifying variables. And yes, it will access even private fields due to some possible issues which might occur otherwise.

I guess your case is the one I didn't actually expect. I guess I will have to add some kind of ignored variables list into the <component> <ui> and <painter> tags inside single style. So you will just need to add a separate style descriptor for your panel specifying background as ignored field there.

This will look something like this:

<skin>
    <style type="panel" id="backgroundPanel" extends="default">
        <component ignoredFields="background" />
    </style>
</skin>

Though that will also require a convenient way to register supported components into StyleManager and a few other improvements. I will add those changes in next update.

@mgarin mgarin added this to the Problematic issues milestone Apr 21, 2014
@mgarin mgarin self-assigned this Apr 21, 2014
@Sciss
Copy link
Contributor Author

Sciss commented Apr 21, 2014

In any case, since your reflection code does check for types (as far as I can see), the problem might be that it does not correctly distinguish between private fields of a class and a sub-class, otherwise it would have found that the type of that field is not java.awt.Color.

You should probably be able to reproduce it like this:

public class Foo extends JPanel {
    private String background = "foo";
}

So can I paste that XML fragment into some resource file to activate it?

@mgarin
Copy link
Owner

mgarin commented Apr 21, 2014

WebLaF doesn't actually check field type when setting value since it would reduce performance a lot. It simply tries to apply value specified in the style to the field in the component instance - it is up to you to provide proper style for each component.

There are some predefined exceptions which may point to your mistakes so you can fix them quickly without digging into WebLaF code or looking for the cause in style. In your case exception is thrown outside of WebLaF but you can clearly see that WebLaF tried to set background field in JPanel-extending class but that field had different type. That happened because there is no separate style for bibliothek.gui.dock.util.BackgroundPanel panel type which would override background field value or simply disable its changes (that should be implemented and not yet available in v1.27).


Field type is checked only when value is being read from the style XML in XStream converter for style object - otherwise I wouldn't be able to understand what am I reading at all. But it is checked in class associated with specified in style type:

<skin>
    <style type="panel" id="backgroundPanel" extends="default">
        <component ignoredFields="background" />
    </style>
</skin>

You can see here type="panel" - that points to SupportedComponent.panel enumeration, which in turn points to JPanel.class. That class is used to check field type in converter. So in your case an additional support required for bibliothek.gui.dock.util.BackgroundPanel class/type which should fix the problem. But custom components style registration is not yet implemented - that is what I am going to add in next update as well.

So, no, you cannot fix this case yet. Example I have provided is just a preview of what you might need to add after the next update. I will explain it again as soon as I implement those features as I am not yet sure how it will function and look like.


As you can see each separate part of WebLaF styling mechanism is pretty simple, but its behavior overall becomes really complicated and might cause unexpected issues. So I am still "in the middle of some calibrations" and any feedback is really appreciated :)

@Sciss
Copy link
Contributor Author

Sciss commented Apr 21, 2014

Ok, thanks for the long explanation. I will see if I can perhaps use another sub-class which re-introduces a dummy background field again.

@mgarin
Copy link
Owner

mgarin commented Apr 21, 2014

That might fix the issue for now, indeed. Anyway, I will be adding those improvements for sure as issues similar to this one might be pretty common.

mgarin added a commit that referenced this issue May 12, 2014
… properties lists

SupportedComponent class refactored
@mgarin
Copy link
Owner

mgarin commented May 12, 2014

I have commited changes considering this case, here is the style you have to add to force skin to skip some specific property:

<style type="panel" id="ignored-background" extends="default">
    <component>
        <background ignored="true" />
    </component>
</style>

For now you cannot add a custom supported component, so you will have to manually install styleId into your component that extends panel:

StyleManager.setStyleId ( component, "ignored-background" );

As soon as custom component support registration will be added you will be able to register specific style types for your own component and skip applying the style for each component instance - default style will be applied automatically.

@mgarin
Copy link
Owner

mgarin commented May 20, 2014

By the way, similar technique can used to ignore any other field in component/ui/painter.
Simply add the ignore line with the field name as the attribute name:

<field-name ignored="true" />

@mgarin
Copy link
Owner

mgarin commented Jun 6, 2014

Have you tried this fix yet by any chance?

@Sciss
Copy link
Contributor Author

Sciss commented Jun 8, 2014

I just tried. It seems to work now, actually without me having to add any xml specification. (The exception has disappeared). I think this issue can be closed.

@mgarin
Copy link
Owner

mgarin commented Jun 8, 2014

Probably some other changes I have made to StyleManager covered this case as well.
In any case, I will close it as soon as v1.28 released.

@Sciss
Copy link
Contributor Author

Sciss commented Jun 8, 2014

Which commit corresponds to v1.28 - the latest one? I will publish again an artifact on Maven Central, so I can use this new version via Maven.

@mgarin
Copy link
Owner

mgarin commented Jun 8, 2014

What do you mean under "corresponds to v1.28"?
New v1.28 update is not yet ready.
If you want a temporary build - simply pick the latest commit.

@Sciss
Copy link
Contributor Author

Sciss commented Jun 8, 2014

Ah, sorry, misread

@mgarin mgarin added the fixed label Jun 26, 2014
@mgarin
Copy link
Owner

mgarin commented Jun 26, 2014

Those changes are now available in v1.28 update:
https://github.com/mgarin/weblaf/releases/tag/v1.28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants