RenderkitUtils and decodeBehaviors

[Brian Leathem]

Hey guys,

Working on RF-10862 [1], I've found that the behaviorSource check in RenderKitUtils is too restrictive.  Specifically the check that the behaviorSource matches the clientId of the component.  In the case of RF-10862, the behaviorSource is the Input nested within the autocomplete component.  My suggested fix is to make the behaviorSource check less restrictive, and replace the equals check [2] with a startsWith check [3].

This indeed fixes the problem with RF-10862.  I'm curious what others thing might be the consequences of such a change.

Feedback appreciated!

Brian

[1] https://issues.jboss.org/browse/RF-10862

[2] https://github.com/richfaces/components/blob/develop/common/ui/src/main/java/org/richfaces/renderkit/RenderKitUtils.java#L509

[3] https://github.com/richfaces/components/blob/feature/RF-10862/common/ui/src/main/java/org/richfaces/renderkit/RenderKitUtils.java#L509

[Brian Leathem]

Thinking about it some more, the startsWith check would give a false positive when checking nested components.  However, if we check for the JSF client ID sperator in the trailing piece, we would be more rigorous.

[Brian Leathem]

I implemented the above check, and pushed the change to the github branch:

https://github.com/richfaces/components/tree/feature/RF-10862

The check is done in the method:

    public static boolean isBehaviorSourceValid(char separatorChar, String behaviorSource, String clientId) {
        if (behaviorSource == null) {
            return false;
        }
        if (!behaviorSource.startsWith(clientId)) {
            return false;
        }
        int suffixStart = behaviorSource.indexOf(clientId);
        int suffixEnd = suffixStart + clientId.length();
        String suffix = behaviorSource.substring(suffixEnd);
        boolean containsSeperatorChar = (suffix.indexOf(separatorChar) >= 0);
        return (! containsSeperatorChar);
    }

[Nick Belaevski]

Brian,

IMHO, this is a problem of autocomplete that should be providing correct behavior context with clientId explicitly set on rendering and utility class should not left changed. For example you can take a look at how Mojarra/MyFaces do this for select many checkbox component.

Alternatively, 'change' event can be called in the context of root component element.

[Ilya Shaikovsky]

First thoughts was about the cases like

<input id="input1"/>

and

<input id="input2"/>

if somehow "input" will be passed as a source (some durty javascript sent just from debug console or something like that) then it potentially could allow the behavior from input1 pretend to be bound to input2. But thats seems too fantasy case. 

And I think that separatorChar check you made is really enough and should be the only problem.

[Brian Leathem]

I remedied RF-10862 by firing an additional event with the ID of the autocomplete component, rather than loosening up the RenderkitUtils decodeBehaviours restrictions.

If more similar issues come up, we can take another look at loosening up the decodeBehaviours restrictions.

[Brian Leathem]

This came up again with RF-12114.  I again resolved the issue by firing secondary events.  We need a more robust solution, as this undobtedly affects other components too.

The DOM event model supports bubbling of events, we should consider supporting a similar bubbling of events within event model of the JSF component tree.  The startWith/separationChar check suggested above seems like a good start at supporting such a bubbling of events, by restricting the "bubbling" of JSF events accross JSF component boundaries.

We should build the required tests to guard against regressions, and make the above change.

[Lukáš Fryč]

Brian, could you write down how you imagine the proposed alghoritm for DOM Event bubbling?