You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Vincent Demay <vi...@anyware-tech.com> on 2007/03/19 12:29:03 UTC

Going from setVisible(false) to setVisible(true) does not work without surounding element

Hi,
    In the current implementation, going from setVisible(false) to 
setVisible(true) in ajax does not work, The current workaround of this 
bug is to surround the element with an other and to rerender the 
surounding element. I found this way not very smart and too frustating 
for the user. We actually need to surounding because nothing is render 
when setVisible is false.

    In https://issues.apache.org/jira/browse/WICKET-365 issue, I propose 
a fix. This fix will simply render a tag with an id and a style 
display:none. I don't think it is very expensive and works well with all 
browsers (IE, FF, Safari) and also in html table (often a problem in 
IE). With this patch it is possible to change the visibility of a 
component in a page without surounding it and adding it to the target.
    This issue has been tagged as won't fix, But I still think it is a 
good stuff

    For exemple  a template(1) with foo.setVisible(false) and 
bar.setVisible(false) will be render as (2). See 
https://issues.apache.org/jira/secure/attachment/12353204/Component.patch.txt

(1)
<div wicket:id="bar">
    <div>Inner</div>
    <p>lsdkqjlqkdsfj</p>
    ... a lot of inner content
</div>
<table>
     <tr>
       <td>
             ....
       </td>
    </tr>
    <tr wicket:id="foo">
       <td>bla bla</td>
    </tr>
</table>



(2)
<div id="bar0" style="display:none"></div>
<table>
     <tr>
       <td>
             ....
       </td>
    </tr>
    <tr id="foo1" style="display:none"></tr>
</table>

I think this patch can make setVisible(false -> true) in ajax smarter 
and more intuitive for user.

WDYT?




Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Frédéric Bertin a écrit :
> Martijn Dashorst wrote:
>> So you mean:
>>
>>    Label l = Label("foo", "hello");
>> renders:
>>    <span wicket:id="foo">hello</span>
>>
>> ... some ajax stuff, or a normal page render:
>>
>>    l.setVisible(false);
>> renders:
>>    <span wicket:id="foo" style="display:none"></span>
> not @wicket:id, but just @id, because the Label should have 
> setOutputMarkupId(true) to be refreshed with ajax
Exactly, the goal of the id on a tag is to make wicket-ajax.js able to 
find it in the dom in order to replace it
>
> Fred
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Frédéric Bertin <fr...@anyware-tech.com>.
Martijn Dashorst wrote:
> So you mean:
>
>    Label l = Label("foo", "hello");
> renders:
>    <span wicket:id="foo">hello</span>
>
> ... some ajax stuff, or a normal page render:
>
>    l.setVisible(false);
> renders:
>    <span wicket:id="foo" style="display:none"></span>
not @wicket:id, but just @id, because the Label should have 
setOutputMarkupId(true) to be refreshed with ajax

Fred


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
Unlike wrapping components in webmarkupcontainer, this should work for
all tags, including <td, <tr, etc.

On 3/19/07, Johan Compagner <jc...@gmail.com> wrote:
> yeah i could live with this solution, if it works for all kind of tags.
>
> johan
>
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >
> > Indeed, it wil fail. but I kind of fail to see practical value of this
> > behavior. To me it looks much more useful to be able to
> > setVisible(false) and later setVisible(true), without having to worry
> > about wrapping the element (which can be rather tricky with element
> > like td).
> >
> > Perhaps we could have a flag on webmarkupcontainer, that would control
> > how should it render when not visible? I mean, one more flag certainly
> > won't harm. And it wouldn't break any of the existing code. All it
> > would do is that when a component is not visible, it will render as
> > <componentTagName style="display:none"
> > id="componentId"></componentTagName>.
> >
> > -Matej
> >
> > On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > Currently everybody assumes (correctly) that the element is completely
> > > removed (Ajax and non-Ajax), i.e. not present in the final markup.
> > > This means that scripts that iterate through the dom, or check for the
> > > document.getElementById() == null will fail if we implement this.
> > >
> > > I *strongly* discourage changing this behavior.
> > >
> > > Martijn
> > >
> > > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > > > Will it? This seems to be actually quite a smart workaround. How
> > > > exactly will this break existing clients? Only thing i'm concerned
> > > > about is the validity of output markup. but imho when we preserve the
> > > > original tag names, e.g. td will render as td, it should be all right.
> > > >
> > > > -Matej
> > > >
> > > > On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > > > So you mean:
> > > > >
> > > > >     Label l = Label("foo", "hello");
> > > > > renders:
> > > > >     <span wicket:id="foo">hello</span>
> > > > >
> > > > > ... some ajax stuff, or a normal page render:
> > > > >
> > > > >     l.setVisible(false);
> > > > > renders:
> > > > >     <span wicket:id="foo" style="display:none"></span>
> > > > > ?
> > > > >
> > > > > This can and will break existing clients in a very nasty manner,
> > > > > because the markup id is still present in the final markup.
> > > > >
> > > > > Martijn
> > > > >
> > > > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > > > > Johan Compagner a écrit :
> > > > > > >> > Also always just rendering the component but use the style to
> > make in
> > > > > > >> > invisible
> > > > > > >> > could be a security problem. So that can't be the default.
> > > > > > >>
> > > > > > >> What do you mean by security problem?
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > If the the component that is  set to none visible is none
> > visible
> > > > > > > because of
> > > > > > > security
> > > > > > > So it has data that never should be send to the browser because
> > the
> > > > > > > user is
> > > > > > > not allowed
> > > > > > > to see it.
> > > > > >
> > > > > > But data is never send to the user because a none visible
> > component will
> > > > > > be render as an empty tag, so without data
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > > > Join the wicket community at irc.freenode.net: ##wicket
> > > > > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > > > > http://wicketframework.org
> > > > >
> > > >
> > >
> > >
> > > --
> > > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > Join the wicket community at irc.freenode.net: ##wicket
> > > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > > http://wicketframework.org
> > >
> >
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Johan Compagner <jc...@gmail.com>.
yeah i could live with this solution, if it works for all kind of tags.

johan


On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>
> Indeed, it wil fail. but I kind of fail to see practical value of this
> behavior. To me it looks much more useful to be able to
> setVisible(false) and later setVisible(true), without having to worry
> about wrapping the element (which can be rather tricky with element
> like td).
>
> Perhaps we could have a flag on webmarkupcontainer, that would control
> how should it render when not visible? I mean, one more flag certainly
> won't harm. And it wouldn't break any of the existing code. All it
> would do is that when a component is not visible, it will render as
> <componentTagName style="display:none"
> id="componentId"></componentTagName>.
>
> -Matej
>
> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > Currently everybody assumes (correctly) that the element is completely
> > removed (Ajax and non-Ajax), i.e. not present in the final markup.
> > This means that scripts that iterate through the dom, or check for the
> > document.getElementById() == null will fail if we implement this.
> >
> > I *strongly* discourage changing this behavior.
> >
> > Martijn
> >
> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > > Will it? This seems to be actually quite a smart workaround. How
> > > exactly will this break existing clients? Only thing i'm concerned
> > > about is the validity of output markup. but imho when we preserve the
> > > original tag names, e.g. td will render as td, it should be all right.
> > >
> > > -Matej
> > >
> > > On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > > So you mean:
> > > >
> > > >     Label l = Label("foo", "hello");
> > > > renders:
> > > >     <span wicket:id="foo">hello</span>
> > > >
> > > > ... some ajax stuff, or a normal page render:
> > > >
> > > >     l.setVisible(false);
> > > > renders:
> > > >     <span wicket:id="foo" style="display:none"></span>
> > > > ?
> > > >
> > > > This can and will break existing clients in a very nasty manner,
> > > > because the markup id is still present in the final markup.
> > > >
> > > > Martijn
> > > >
> > > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > > > Johan Compagner a écrit :
> > > > > >> > Also always just rendering the component but use the style to
> make in
> > > > > >> > invisible
> > > > > >> > could be a security problem. So that can't be the default.
> > > > > >>
> > > > > >> What do you mean by security problem?
> > > > > >
> > > > > >
> > > > > >
> > > > > > If the the component that is  set to none visible is none
> visible
> > > > > > because of
> > > > > > security
> > > > > > So it has data that never should be send to the browser because
> the
> > > > > > user is
> > > > > > not allowed
> > > > > > to see it.
> > > > >
> > > > > But data is never send to the user because a none visible
> component will
> > > > > be render as an empty tag, so without data
> > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > > Join the wicket community at irc.freenode.net: ##wicket
> > > > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > > > http://wicketframework.org
> > > >
> > >
> >
> >
> > --
> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > Join the wicket community at irc.freenode.net: ##wicket
> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > http://wicketframework.org
> >
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
Indeed, it wil fail. but I kind of fail to see practical value of this
behavior. To me it looks much more useful to be able to
setVisible(false) and later setVisible(true), without having to worry
about wrapping the element (which can be rather tricky with element
like td).

Perhaps we could have a flag on webmarkupcontainer, that would control
how should it render when not visible? I mean, one more flag certainly
won't harm. And it wouldn't break any of the existing code. All it
would do is that when a component is not visible, it will render as
<componentTagName style="display:none"
id="componentId"></componentTagName>.

-Matej

On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> Currently everybody assumes (correctly) that the element is completely
> removed (Ajax and non-Ajax), i.e. not present in the final markup.
> This means that scripts that iterate through the dom, or check for the
> document.getElementById() == null will fail if we implement this.
>
> I *strongly* discourage changing this behavior.
>
> Martijn
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > Will it? This seems to be actually quite a smart workaround. How
> > exactly will this break existing clients? Only thing i'm concerned
> > about is the validity of output markup. but imho when we preserve the
> > original tag names, e.g. td will render as td, it should be all right.
> >
> > -Matej
> >
> > On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > So you mean:
> > >
> > >     Label l = Label("foo", "hello");
> > > renders:
> > >     <span wicket:id="foo">hello</span>
> > >
> > > ... some ajax stuff, or a normal page render:
> > >
> > >     l.setVisible(false);
> > > renders:
> > >     <span wicket:id="foo" style="display:none"></span>
> > > ?
> > >
> > > This can and will break existing clients in a very nasty manner,
> > > because the markup id is still present in the final markup.
> > >
> > > Martijn
> > >
> > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > > Johan Compagner a écrit :
> > > > >> > Also always just rendering the component but use the style to make in
> > > > >> > invisible
> > > > >> > could be a security problem. So that can't be the default.
> > > > >>
> > > > >> What do you mean by security problem?
> > > > >
> > > > >
> > > > >
> > > > > If the the component that is  set to none visible is none visible
> > > > > because of
> > > > > security
> > > > > So it has data that never should be send to the browser because the
> > > > > user is
> > > > > not allowed
> > > > > to see it.
> > > >
> > > > But data is never send to the user because a none visible component will
> > > > be render as an empty tag, so without data
> > > >
> > > >
> > >
> > >
> > > --
> > > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > Join the wicket community at irc.freenode.net: ##wicket
> > > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > > http://wicketframework.org
> > >
> >
>
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Igor Vaynberg <ig...@gmail.com>.
On 3/19/07, Frédéric Bertin <fr...@anyware-tech.com> wrote:
>
> setOutputMarkupId(true), because it is simply useless, no?


no that the thing it is not useless. all its saying is that i want the id
output when the component tag renders. it says nothing about the situation
when the component is not visible.

-igor

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Frédéric Bertin <fr...@anyware-tech.com>.
Igor Vaynberg wrote:
> im not a big fan of having an application-wide setting for this. it will
> make it harder to integrate with 3rd party components that werent written
> with the setting in mind because it pretty much gives you an option of
> ignoring it by assuming a default.
>
> better make it explicit, and it doesnt add any extra work. instead of
> calling setoutputmarkupid(true) just call setoutputmarkuptag() or 
> maybe even
> call it setoutputmarkuptagandid()
well, when calling setOutputMarkupId(true), setOutputMarkupTag(true) is 
implicit, because an attribute doesn't exist without a parent tag ;-)
this makes me think that the solution is just there, in Vincent's patch :)
Indeed, I assume that all existing uses of setVisible(false) don't use 
setOutputMarkupId(true), because it is simply useless, no?
If this is true, the patch won't break anything, because it only changes 
the behaviour when setOuputMarkuId is set to true.

Fred
>
> -igor
>
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>>
>> Well, I'm also -1 for having this behavior off for default.
>> But, i think it can be quite useful for couple of cases, therefore i
>> think there should be support for that.
>>
>> I think what we need is an application setting, for default behavior,
>> and then a getter for each component.
>> e.g. boolean Component.getRenderEmptyTagOnNotVisible()
>> that would by default return the application settings value.
>>
>> This would cost us nothing, as there is not even flag needed for it.
>> And would be versatile enough, to only enable the behavior on
>> components where it really make sense and doesn't break anything. But
>> this doesn't mean i couldn't live with a component flag of course :)
>>
>> -Matej
>>
>> On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
>> > what you need is a different method. instead of 
>> setoutputmarkupid(true)
>> you
>> > need something that will do
>> >
>> > setoutputmarkuptag(boolean b) {
>> >   if (b) {
>> >      setoutputmarkupid(true);
>> >      setflag(outputmarkuptag,true);
>> >    } else {
>> >        setflag(outputmarkuptag,false);
>> >        //not sure if we should undo setoutputmarkupid here
>> >     }
>> > }
>> >
>> > now this will solve the problems outlined so far
>> >
>> > btw the latest patch attached to the issue is bad because it executes
>> > behaviors which should not happen.
>> >
>> > what i am worried about are javascript libraries and css. does css 3
>> have
>> > support for odd/even selectors? that will break. this will even break
>> css 2
>> > first child selector if that happens to be invisible because i dont
>> think it
>> > checks for the display attribute. it will also break  3rd party
>> javascript
>> > libs that dont check for display attrs.
>> >
>> > i see how this makes life easier, but it also has a potential to be
>> evil.
>> > that is why i closed the issue as wont-fix and told vincent to 
>> bring the
>> > discussion here on the list before we go any further.
>> >
>> > -igor
>> >
>> >
>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> > >
>> > > I don't like it. I don't think deprecating set/isVisible is the 
>> way to
>> > > go. Plus there are other reasons: As now we use one flag for visible
>> > > status. With your approach you'd require an enum, int, byte or
>> > > something similiar, that takes more space then just one bit in 
>> flags.
>> > >
>> > > I think we can just make a getter, that by default returns the value
>> > > from application settings. And you can override that for your
>> > > component, if you want something different that what's set in
>> > > application settings.
>> > >
>> > > -Matej
>> > >
>> > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> > > > Frédéric Bertin a écrit :
>> > > > > Martijn Dashorst wrote:
>> > > > >>> it seems taht this kind of construction is used to make
>> workaround
>> > > of
>> > > > >>> the bug. Is'n it?
>> > > > >>
>> > > > >> First, what bug? I don't know that this is a bug? I thought we
>> are
>> > > > >> discussing a feature here. Secondly, this is not a workaround,
>> but
>> > > > >> creating client side code based on a API contract:
>> setVisible(false)
>> > > > >> removes the component markup completely, including its tags, 
>> from
>> the
>> > > > >> final markup.
>> > > > > ok, then there's a need for 2 different mechanisms.
>> > > > > One to switch a component visibility. This one should be
>> *reversible*
>> > > > > (in ajax or not), and then it should always output a tag with an
>> id
>> > > > > attribute (actually, can be done only when setOutputMarkupId is
>> set to
>> > > > > true).
>> > > > >
>> > > > > Another to render or not a component in the output markup. This
>> one
>> > > > > should be documented as *not reversible*. This is the current
>> > > > > setVisible implementation.
>> > > > >
>> > > > > wdyt?
>> > > >
>> > > > +1
>> > > > What about keeping current behavior on setVisible (deprecated) and
>> > > > add a method setVisibility :
>> > > >     - none : render nothing
>> > > >     - visible : render all
>> > > >     - invisible : render only container tag with 
>> style:display-none
>> > > >
>> > > > Add in documentation
>> > > > none: can not become visible in ajax
>> > > > in visible: can
>> > > >
>> > > > I think it will match to all use case, no ?
>> > > > >
>> > > > >
>> > > > >
>> > > > >>
>> > > > >> It is based on the assumption that some element is *NOT* 
>> present
>> at
>> > > > >> all. Your change will invert that behavior, and in such a way
>> that it
>> > > > >> is only detectable by debugging your javascript. Not 
>> something I
>> > > > >> enjoy, nor 95% of the development community.
>> > > > >>
>> > > > >> You must understand that this is a major api break, not 
>> something
>> > > > >> minor. This is not detectable by a compiler. You *will* break
>> > > existing
>> > > > >> scripts, pages and applications in a non-obvious way. Silent
>> failures
>> > > > >> are something we try to avoid at all cost.
>> > > > >>
>> > > > >> Martijn
>> > > > >>
>> > > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> 
>> wrote:
>> > > > >>> Martijn Dashorst a écrit :
>> > > > >>> > Currently everybody assumes (correctly) that the element is
>> > > > >>> completely
>> > > > >>> > removed (Ajax and non-Ajax)
>> > > > >>> Yes of course, but it is the same for all workarounds ;)
>> > > > >>> When to change servlet to filter, users have to change 
>> their web
>> > > xml.
>> > > > >>> Each time you change something users have to adapt their code
>> > > > >>> > i.e. not present in the final markup.
>> > > > >>> > This means that scripts that iterate through the dom, or 
>> check
>> for
>> > > > >>> the
>> > > > >>> > document.getElementById() == null will fail if we implement
>> this.
>> > > > >>> >
>> > > > >>> it seems taht this kind of construction is used to make
>> workaround
>> > > of
>> > > > >>> the bug. Is'n it?
>> > > > >>> > I *strongly* discourage changing this behavior.
>> > > > >>> >
>> > > > >>> > Martijn
>> > > > >>> >
>> > > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> > > > >>> >> Will it? This seems to be actually quite a smart 
>> workaround.
>> How
>> > > > >>> >> exactly will this break existing clients? Only thing i'm
>> > > concerned
>> > > > >>> >> about is the validity of output markup. but imho when we
>> preserve
>> > > > >>> the
>> > > > >>> >> original tag names, e.g. td will render as td, it should be
>> all
>> > > > >>> right.
>> > > > >>> >>
>> > > > >>> >> -Matej
>> > > > >>> >>
>> > > > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
>> wrote:
>> > > > >>> >> > So you mean:
>> > > > >>> >> >
>> > > > >>> >> >     Label l = Label("foo", "hello");
>> > > > >>> >> > renders:
>> > > > >>> >> >     <span wicket:id="foo">hello</span>
>> > > > >>> >> >
>> > > > >>> >> > ... some ajax stuff, or a normal page render:
>> > > > >>> >> >
>> > > > >>> >> >     l.setVisible(false);
>> > > > >>> >> > renders:
>> > > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
>> > > > >>> >> > ?
>> > > > >>> >> >
>> > > > >>> >> > This can and will break existing clients in a very nasty
>> > > manner,
>> > > > >>> >> > because the markup id is still present in the final 
>> markup.
>> > > > >>> >> >
>> > > > >>> >> > Martijn
>> > > > >>> >> >
>> > > > >>> >> > On 3/19/07, Vincent Demay 
>> <vi...@anyware-tech.com>
>> > > wrote:
>> > > > >>> >> > > Johan Compagner a écrit :
>> > > > >>> >> > > >> > Also always just rendering the component but 
>> use the
>> > > style
>> > > > >>> >> to make in
>> > > > >>> >> > > >> > invisible
>> > > > >>> >> > > >> > could be a security problem. So that can't be the
>> > > default.
>> > > > >>> >> > > >>
>> > > > >>> >> > > >> What do you mean by security problem?
>> > > > >>> >> > > >
>> > > > >>> >> > > >
>> > > > >>> >> > > >
>> > > > >>> >> > > > If the the component that is  set to none visible is
>> none
>> > > > >>> visible
>> > > > >>> >> > > > because of
>> > > > >>> >> > > > security
>> > > > >>> >> > > > So it has data that never should be send to the 
>> browser
>> > > > >>> because
>> > > > >>> >> the
>> > > > >>> >> > > > user is
>> > > > >>> >> > > > not allowed
>> > > > >>> >> > > > to see it.
>> > > > >>> >> > >
>> > > > >>> >> > > But data is never send to the user because a none 
>> visible
>> > > > >>> >> component will
>> > > > >>> >> > > be render as an empty tag, so without data
>> > > > >>> >> > >
>> > > > >>> >> > >
>> > > > >>> >> >
>> > > > >>> >> >
>> > > > >>> >> > --
>> > > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> > > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> > > > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
>> now!
>> > > > >>> >> > http://wicketframework.org
>> > > > >>> >> >
>> > > > >>> >>
>> > > > >>> >
>> > > > >>> >
>> > > > >>>
>> > > > >>>
>> > > > >>
>> > > > >>
>> > > > >
>> > > > >
>> > > >
>> > > >
>> > >
>> >
>>
>


-- 
Frédéric BERTIN
Contrôle industriel
ANYWARE TECHNOLOGIES
Tél (direct) : +33 (0)5.61.00.06.41
Tél (standard) : +33 (0)5.61.00.52.90
Fax : +33 (0)5 61 00 51 46
http://www.anyware-tech.com


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
After all this discuss,

I made again a patch using Igor method : setoutputmarkuptag
see 
https://issues.apache.org/jira/secure/attachment/12353976/Component.java.365.patch.txt

WDYT? I hope it will be ok for all ;)

--
Vincent

Eelco Hillenius a écrit :
>> What? Why should the inner component render when parent is not
>> visible? This just doesn't make sense.
>
> +1
>
> Eelco


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Eelco Hillenius <ee...@gmail.com>.
> What? Why should the inner component render when parent is not
> visible? This just doesn't make sense.

+1

Eelco

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
What? Why should the inner component render when parent is not
visible? This just doesn't make sense.

If you have two nested components and both are not visible, only the
outer will render - as an empty tag. If you want to show the inner,
you'll have to also show the outer one.

-Matej

On 3/20/07, Martijn Dashorst <ma...@gmail.com> wrote:
> I know that, but that is not what a user expects... This patch wants
> to make ajax visibilitiy changes easier and more transparent. But this
> case doesn't work.
>
> Think from a framework perspective instead of just pushing the patch.
> What can break, what doesn't work, what else have we forgotten? Play
> advocate of the devil.
>
> Now if we don't render child component tags, then the check
> document.getElementById('foo').style.display == 'none' fails. So we
> now introduce ternairy logic. An element can be hidden or not present
> at all. But before this patch it was just not available.
>
> How does this functionality work with security? Does
> setOutputMarkupTag work regardless of authorization? If I'm not
> authorized, does the tag render? Or not?
>
> Martijn
>
> On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > Martijn Dashorst a écrit :
> > > No, because when you set output markuptag on the link *AND* on the
> > > label, what should it do? Those are in conflict. And don't work as
> > > advertised. This is completely going against what we designed
> > > visibility to do: not render markup.
> > If you put setVisible(false) & setOutputMarkupTag(true) on both
> > component, only the parent one will be render :
> >
> > foo = new Link("foo");
> > foo.setVisible(false);
> > foo.setOutputMarkupTag(true)
> > bar =  new label("bar");
> > bar.setVisible(false);
> > bar.setOutputMarkupTag(true)
> > add(foo);
> > foo.add(bar);
> >
> > you will have :
> > <a id="foo0"></a>
> >
> > have a look in patch : markupStream.skipComponent(); will skip child
> > components
> >
> > --
> > Vincent
> >
> >
> > >
> > > When I do this:
> > >
> > > new Link("foo")
> > >            .add(new Label("bar", "foobar"))
> > >            .setOutputWhatever(true)
> > >            .setVisible(false);
> > >
> > > I expect this:
> > >
> > > <a href="#" wicket:id="foo" id="foo" style="display:none"></a>
> > >
> > > And NOT:
> > >
> > > <a href="#" wicket:id="foo" id="foo" style="display:none">
> > >    <span wicket:id="bar" style="display:none"></span>
> > > </a>
> > >
> > > Martijn
> > >
> > > On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > >> Martijn Dashorst a écrit :
> > >> > One of my major pain points here is not discussed yet. And that is
> > >> > nested invisible components.
> > >> >
> > >> > <a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>
> > >> >
> > >> > new Link("foo").add(new Label("bar")).setVisible(false)
> > >> >
> > >> >
> > >> > ...
> > >> > onAjaxThing(target) {
> > >> >    bar.setVisible(true);
> > >> >    target.add(bar);
> > >> > }
> > >> >
> > >> > In both cases this is impossible, however, from what I get from this
> > >> > discussion is that we want to make it magical that this is possible.
> > >> It is no so much magical because you need to call the
> > >> setoutputmarkuptag(true) to do that and you can also revert it when you
> > >> want calling
> > >> setoutputmarkuptag(false)
> > >> >
> > >> > Either way, this is the next thing that comes up, right after ListView
> > >> > and Repeaters...
> > >> No problem because the default behavior is the same as today
> > >> (setoutputmarkuptag(false) by default)
> > >> >
> > >> > And what do we do about components that are not visible because the
> > >> > user is not authorized? Do we generate the tags or not?
> > >> With the last implementation coming out from our discuss is
> > >> If you explicitly called setoutputmarkuptag(true) on your bar component
> > >> yes else no. So you can choose what you want to do?
> > >> >
> > >> > The more I think about it, the more magic is needed, the less I like
> > >> > the idea.
> > >> >
> > >> > Martijn
> > >> >
> > >> --
> > >> Vincent
> > >>
> > >
> > >
> >
> >
>
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Martijn Dashorst a écrit :
> I know that, but that is not what a user expects... This patch wants
> to make ajax visibilitiy changes easier and more transparent. But this
> case doesn't work.
>
> Think from a framework perspective instead of just pushing the patch.
> What can break, what doesn't work, what else have we forgotten? Play
> advocate of the devil.
>
> Now if we don't render child component tags, then the check
> document.getElementById('foo').style.display == 'none' fails. So we
> now introduce ternairy logic. An element can be hidden or not present
> at all. But before this patch it was just not available.
Yes but i think ternairy logic is explicitally introduced by user 
(calling setOutputMarkupTag(true)). In other case, the logic is the same 
as before
so user know that and can adapt his js.
>
> How does this functionality work with security? Does
> setOutputMarkupTag work regardless of authorization? If I'm not
> authorized, does the tag render? Or not?
Even if the tag is rendered, it will be empty. Will it cause a security 
problem? : it is a tag without any data in it
>
> Martijn
>
> On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> Martijn Dashorst a écrit :
>> > No, because when you set output markuptag on the link *AND* on the
>> > label, what should it do? Those are in conflict. And don't work as
>> > advertised. This is completely going against what we designed
>> > visibility to do: not render markup.
>> If you put setVisible(false) & setOutputMarkupTag(true) on both
>> component, only the parent one will be render :
>>
>> foo = new Link("foo");
>> foo.setVisible(false);
>> foo.setOutputMarkupTag(true)
>> bar =  new label("bar");
>> bar.setVisible(false);
>> bar.setOutputMarkupTag(true)
>> add(foo);
>> foo.add(bar);
>>
>> you will have :
>> <a id="foo0"></a>
>>
>> have a look in patch : markupStream.skipComponent(); will skip child
>> components
>>
>> -- 
>> Vincent
>>
>>
>> >
>> > When I do this:
>> >
>> > new Link("foo")
>> >            .add(new Label("bar", "foobar"))
>> >            .setOutputWhatever(true)
>> >            .setVisible(false);
>> >
>> > I expect this:
>> >
>> > <a href="#" wicket:id="foo" id="foo" style="display:none"></a>
>> >
>> > And NOT:
>> >
>> > <a href="#" wicket:id="foo" id="foo" style="display:none">
>> >    <span wicket:id="bar" style="display:none"></span>
>> > </a>
>> >
>> > Martijn
>> >
>> > On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> >> Martijn Dashorst a écrit :
>> >> > One of my major pain points here is not discussed yet. And that is
>> >> > nested invisible components.
>> >> >
>> >> > <a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>
>> >> >
>> >> > new Link("foo").add(new Label("bar")).setVisible(false)
>> >> >
>> >> >
>> >> > ...
>> >> > onAjaxThing(target) {
>> >> >    bar.setVisible(true);
>> >> >    target.add(bar);
>> >> > }
>> >> >
>> >> > In both cases this is impossible, however, from what I get from 
>> this
>> >> > discussion is that we want to make it magical that this is 
>> possible.
>> >> It is no so much magical because you need to call the
>> >> setoutputmarkuptag(true) to do that and you can also revert it 
>> when you
>> >> want calling
>> >> setoutputmarkuptag(false)
>> >> >
>> >> > Either way, this is the next thing that comes up, right after 
>> ListView
>> >> > and Repeaters...
>> >> No problem because the default behavior is the same as today
>> >> (setoutputmarkuptag(false) by default)
>> >> >
>> >> > And what do we do about components that are not visible because the
>> >> > user is not authorized? Do we generate the tags or not?
>> >> With the last implementation coming out from our discuss is
>> >> If you explicitly called setoutputmarkuptag(true) on your bar 
>> component
>> >> yes else no. So you can choose what you want to do?
>> >> >
>> >> > The more I think about it, the more magic is needed, the less I 
>> like
>> >> > the idea.
>> >> >
>> >> > Martijn
>> >> >
>> >> --
>> >> Vincent
>> >>
>> >
>> >
>>
>>
>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
I know that, but that is not what a user expects... This patch wants
to make ajax visibilitiy changes easier and more transparent. But this
case doesn't work.

Think from a framework perspective instead of just pushing the patch.
What can break, what doesn't work, what else have we forgotten? Play
advocate of the devil.

Now if we don't render child component tags, then the check
document.getElementById('foo').style.display == 'none' fails. So we
now introduce ternairy logic. An element can be hidden or not present
at all. But before this patch it was just not available.

How does this functionality work with security? Does
setOutputMarkupTag work regardless of authorization? If I'm not
authorized, does the tag render? Or not?

Martijn

On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> Martijn Dashorst a écrit :
> > No, because when you set output markuptag on the link *AND* on the
> > label, what should it do? Those are in conflict. And don't work as
> > advertised. This is completely going against what we designed
> > visibility to do: not render markup.
> If you put setVisible(false) & setOutputMarkupTag(true) on both
> component, only the parent one will be render :
>
> foo = new Link("foo");
> foo.setVisible(false);
> foo.setOutputMarkupTag(true)
> bar =  new label("bar");
> bar.setVisible(false);
> bar.setOutputMarkupTag(true)
> add(foo);
> foo.add(bar);
>
> you will have :
> <a id="foo0"></a>
>
> have a look in patch : markupStream.skipComponent(); will skip child
> components
>
> --
> Vincent
>
>
> >
> > When I do this:
> >
> > new Link("foo")
> >            .add(new Label("bar", "foobar"))
> >            .setOutputWhatever(true)
> >            .setVisible(false);
> >
> > I expect this:
> >
> > <a href="#" wicket:id="foo" id="foo" style="display:none"></a>
> >
> > And NOT:
> >
> > <a href="#" wicket:id="foo" id="foo" style="display:none">
> >    <span wicket:id="bar" style="display:none"></span>
> > </a>
> >
> > Martijn
> >
> > On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >> Martijn Dashorst a écrit :
> >> > One of my major pain points here is not discussed yet. And that is
> >> > nested invisible components.
> >> >
> >> > <a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>
> >> >
> >> > new Link("foo").add(new Label("bar")).setVisible(false)
> >> >
> >> >
> >> > ...
> >> > onAjaxThing(target) {
> >> >    bar.setVisible(true);
> >> >    target.add(bar);
> >> > }
> >> >
> >> > In both cases this is impossible, however, from what I get from this
> >> > discussion is that we want to make it magical that this is possible.
> >> It is no so much magical because you need to call the
> >> setoutputmarkuptag(true) to do that and you can also revert it when you
> >> want calling
> >> setoutputmarkuptag(false)
> >> >
> >> > Either way, this is the next thing that comes up, right after ListView
> >> > and Repeaters...
> >> No problem because the default behavior is the same as today
> >> (setoutputmarkuptag(false) by default)
> >> >
> >> > And what do we do about components that are not visible because the
> >> > user is not authorized? Do we generate the tags or not?
> >> With the last implementation coming out from our discuss is
> >> If you explicitly called setoutputmarkuptag(true) on your bar component
> >> yes else no. So you can choose what you want to do?
> >> >
> >> > The more I think about it, the more magic is needed, the less I like
> >> > the idea.
> >> >
> >> > Martijn
> >> >
> >> --
> >> Vincent
> >>
> >
> >
>
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Martijn Dashorst a écrit :
> No, because when you set output markuptag on the link *AND* on the
> label, what should it do? Those are in conflict. And don't work as
> advertised. This is completely going against what we designed
> visibility to do: not render markup.
If you put setVisible(false) & setOutputMarkupTag(true) on both 
component, only the parent one will be render :

foo = new Link("foo");
foo.setVisible(false);
foo.setOutputMarkupTag(true)
bar =  new label("bar");
bar.setVisible(false);
bar.setOutputMarkupTag(true)
add(foo);
foo.add(bar);

you will have :
<a id="foo0"></a>

have a look in patch : markupStream.skipComponent(); will skip child 
components

--
Vincent


>
> When I do this:
>
> new Link("foo")
>            .add(new Label("bar", "foobar"))
>            .setOutputWhatever(true)
>            .setVisible(false);
>
> I expect this:
>
> <a href="#" wicket:id="foo" id="foo" style="display:none"></a>
>
> And NOT:
>
> <a href="#" wicket:id="foo" id="foo" style="display:none">
>    <span wicket:id="bar" style="display:none"></span>
> </a>
>
> Martijn
>
> On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> Martijn Dashorst a écrit :
>> > One of my major pain points here is not discussed yet. And that is
>> > nested invisible components.
>> >
>> > <a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>
>> >
>> > new Link("foo").add(new Label("bar")).setVisible(false)
>> >
>> >
>> > ...
>> > onAjaxThing(target) {
>> >    bar.setVisible(true);
>> >    target.add(bar);
>> > }
>> >
>> > In both cases this is impossible, however, from what I get from this
>> > discussion is that we want to make it magical that this is possible.
>> It is no so much magical because you need to call the
>> setoutputmarkuptag(true) to do that and you can also revert it when you
>> want calling
>> setoutputmarkuptag(false)
>> >
>> > Either way, this is the next thing that comes up, right after ListView
>> > and Repeaters...
>> No problem because the default behavior is the same as today
>> (setoutputmarkuptag(false) by default)
>> >
>> > And what do we do about components that are not visible because the
>> > user is not authorized? Do we generate the tags or not?
>> With the last implementation coming out from our discuss is
>> If you explicitly called setoutputmarkuptag(true) on your bar component
>> yes else no. So you can choose what you want to do?
>> >
>> > The more I think about it, the more magic is needed, the less I like
>> > the idea.
>> >
>> > Martijn
>> >
>> -- 
>> Vincent
>>
>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
No, because when you set output markuptag on the link *AND* on the
label, what should it do? Those are in conflict. And don't work as
advertised. This is completely going against what we designed
visibility to do: not render markup.

When I do this:

new Link("foo")
            .add(new Label("bar", "foobar"))
            .setOutputWhatever(true)
            .setVisible(false);

I expect this:

<a href="#" wicket:id="foo" id="foo" style="display:none"></a>

And NOT:

<a href="#" wicket:id="foo" id="foo" style="display:none">
    <span wicket:id="bar" style="display:none"></span>
</a>

Martijn

On 3/20/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> Martijn Dashorst a écrit :
> > One of my major pain points here is not discussed yet. And that is
> > nested invisible components.
> >
> > <a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>
> >
> > new Link("foo").add(new Label("bar")).setVisible(false)
> >
> >
> > ...
> > onAjaxThing(target) {
> >    bar.setVisible(true);
> >    target.add(bar);
> > }
> >
> > In both cases this is impossible, however, from what I get from this
> > discussion is that we want to make it magical that this is possible.
> It is no so much magical because you need to call the
> setoutputmarkuptag(true) to do that and you can also revert it when you
> want calling
> setoutputmarkuptag(false)
> >
> > Either way, this is the next thing that comes up, right after ListView
> > and Repeaters...
> No problem because the default behavior is the same as today
> (setoutputmarkuptag(false) by default)
> >
> > And what do we do about components that are not visible because the
> > user is not authorized? Do we generate the tags or not?
> With the last implementation coming out from our discuss is
> If you explicitly called setoutputmarkuptag(true) on your bar component
> yes else no. So you can choose what you want to do?
> >
> > The more I think about it, the more magic is needed, the less I like
> > the idea.
> >
> > Martijn
> >
> --
> Vincent
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Johan Compagner <jc...@gmail.com>.
new Link("foo").add(new Label("bar")).setVisible(false);

(so the link/outer componetn is not visible)

then this:

onAjaxThing(target) {
    bar.setVisible(true);
    target.add(bar);
}


(setting suddenly the inner component to visible, which was already visible
by itself so this is a NOP)

And adding that to the target shouldn't do one thing.. If that is currently
the case its an error
because a component should only  render itself if itself and all its parent
are visible.
So the target.add(bar) should result in no output at all because the parent
is not visible.

if you did this:


 onAjaxThing(target) {
    foo.setVisible(true);
    target.add(bar);
}

then that is a programming error,

this should generate all the markup:

onAjaxThing(target) {
    foo.setVisible(true);
    target.add(foo);
}

and this

onAjaxThing(target) {
    foo.setVisible(true);
    bar.setVisible(false);
    target.add(foo);
}

should generate the link, but with an empty span inside it, so that
afterwards this works:


onAjaxThing(target) {
    bar.setVisible(true);
    target.add(bar);
}

i like this approach it is much more self explaining then what we have now

Currently in my project is have a special setComponentVisible(boolean) on
all my components
that sets a special visible flag so that i can generate display:none or not.
So that i can do the thing i want here. Because i really don't want
components to not be there..
(and in our setup i really can't wrap every component in a container..)

johan


On 3/20/07, Martijn Dashorst <ma...@gmail.com> wrote:
>
> One of my major pain points here is not discussed yet. And that is
> nested invisible components.
>
> <a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>
>
> new Link("foo").add(new Label("bar")).setVisible(false)
>
>
> ...
> onAjaxThing(target) {
>     bar.setVisible(true);
>     target.add(bar);
> }
>
> In both cases this is impossible, however, from what I get from this
> discussion is that we want to make it magical that this is possible.
>
> Either way, this is the next thing that comes up, right after ListView
> and Repeaters...
>
> And what do we do about components that are not visible because the
> user is not authorized? Do we generate the tags or not?
>
> The more I think about it, the more magic is needed, the less I like the
> idea.
>
> Martijn
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Martijn Dashorst a écrit :
> One of my major pain points here is not discussed yet. And that is
> nested invisible components.
>
> <a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>
>
> new Link("foo").add(new Label("bar")).setVisible(false)
>
>
> ...
> onAjaxThing(target) {
>    bar.setVisible(true);
>    target.add(bar);
> }
>
> In both cases this is impossible, however, from what I get from this
> discussion is that we want to make it magical that this is possible.
It is no so much magical because you need to call the 
setoutputmarkuptag(true) to do that and you can also revert it when you 
want calling
setoutputmarkuptag(false)
>
> Either way, this is the next thing that comes up, right after ListView
> and Repeaters...
No problem because the default behavior is the same as today 
(setoutputmarkuptag(false) by default)
>
> And what do we do about components that are not visible because the
> user is not authorized? Do we generate the tags or not?
With the last implementation coming out from our discuss is
If you explicitly called setoutputmarkuptag(true) on your bar component 
yes else no. So you can choose what you want to do?
>
> The more I think about it, the more magic is needed, the less I like 
> the idea.
>
> Martijn
>
--
Vincent

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
One of my major pain points here is not discussed yet. And that is
nested invisible components.

<a href="#" wicket:id="foo"><span wicket:id="bar"></span></a>

new Link("foo").add(new Label("bar")).setVisible(false)


...
onAjaxThing(target) {
    bar.setVisible(true);
    target.add(bar);
}

In both cases this is impossible, however, from what I get from this
discussion is that we want to make it magical that this is possible.

Either way, this is the next thing that comes up, right after ListView
and Repeaters...

And what do we do about components that are not visible because the
user is not authorized? Do we generate the tags or not?

The more I think about it, the more magic is needed, the less I like the idea.

Martijn

-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Frédéric Bertin <fr...@anyware-tech.com>.
Igor Vaynberg wrote:
>
>> As a user, I want to refresh my component with Ajax. I don't care if, to
>> achieve that, Wicket needs a tag, an id, or anything else.
>
>
> which is what i proposed below in my code snippet
yes, indeed. My point was just about giving methods a feature-related 
name (setAjaxRefresheable), rather than a technical 
implementation-related one (setOutputMarkupTagAndId).

It's just nitpicking, but I remember being a little bit surprised when 
first using Ajax with Wicket by the fact that I had to call the method 
setOutputMarkupId(true). What I found strange was really the method 
name. I understood why it was called like this because I know the 
mechanism behind, but I don't know, it's somewhat like using 
setBackgroundGreyedAndNotEditable instead of setDisabled (exaggerating a 
lot). Don't say we should rename the "technical" method, but just create 
a new more user friendly one, easier to find when looking for the 
magical method using code completion. While you have a one to one 
relation between them, perhaps it's over-coded, but as soon as this 
changes, it's maybe worth doing it.

Anyway, whatever the name, I'm +1 for the solution you proposed.

Thanks.

Fred

>
> -igor
>
>
>
> Does it make sense?
>>
>> Fred
>>
>> Igor Vaynberg wrote:
>> > im not a big fan of having an application-wide setting for this. it 
>> will
>> > make it harder to integrate with 3rd party components that werent
>> written
>> > with the setting in mind because it pretty much gives you an option of
>> > ignoring it by assuming a default.
>> >
>> > better make it explicit, and it doesnt add any extra work. instead of
>> > calling setoutputmarkupid(true) just call setoutputmarkuptag() or
>> > maybe even
>> > call it setoutputmarkuptagandid()
>> >
>> > -igor
>> >
>> >
>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >>
>> >> Well, I'm also -1 for having this behavior off for default.
>> >> But, i think it can be quite useful for couple of cases, therefore i
>> >> think there should be support for that.
>> >>
>> >> I think what we need is an application setting, for default behavior,
>> >> and then a getter for each component.
>> >> e.g. boolean Component.getRenderEmptyTagOnNotVisible()
>> >> that would by default return the application settings value.
>> >>
>> >> This would cost us nothing, as there is not even flag needed for it.
>> >> And would be versatile enough, to only enable the behavior on
>> >> components where it really make sense and doesn't break anything. But
>> >> this doesn't mean i couldn't live with a component flag of course :)
>> >>
>> >> -Matej
>> >>
>> >> On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
>> >> > what you need is a different method. instead of
>> >> setoutputmarkupid(true)
>> >> you
>> >> > need something that will do
>> >> >
>> >> > setoutputmarkuptag(boolean b) {
>> >> >   if (b) {
>> >> >      setoutputmarkupid(true);
>> >> >      setflag(outputmarkuptag,true);
>> >> >    } else {
>> >> >        setflag(outputmarkuptag,false);
>> >> >        //not sure if we should undo setoutputmarkupid here
>> >> >     }
>> >> > }
>> >> >
>> >> > now this will solve the problems outlined so far
>> >> >
>> >> > btw the latest patch attached to the issue is bad because it 
>> executes
>> >> > behaviors which should not happen.
>> >> >
>> >> > what i am worried about are javascript libraries and css. does 
>> css 3
>> >> have
>> >> > support for odd/even selectors? that will break. this will even 
>> break
>> >> css 2
>> >> > first child selector if that happens to be invisible because i dont
>> >> think it
>> >> > checks for the display attribute. it will also break  3rd party
>> >> javascript
>> >> > libs that dont check for display attrs.
>> >> >
>> >> > i see how this makes life easier, but it also has a potential to be
>> >> evil.
>> >> > that is why i closed the issue as wont-fix and told vincent to
>> >> bring the
>> >> > discussion here on the list before we go any further.
>> >> >
>> >> > -igor
>> >> >
>> >> >
>> >> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >> > >
>> >> > > I don't like it. I don't think deprecating set/isVisible is the
>> >> way to
>> >> > > go. Plus there are other reasons: As now we use one flag for
>> visible
>> >> > > status. With your approach you'd require an enum, int, byte or
>> >> > > something similiar, that takes more space then just one bit in
>> >> flags.
>> >> > >
>> >> > > I think we can just make a getter, that by default returns the
>> value
>> >> > > from application settings. And you can override that for your
>> >> > > component, if you want something different that what's set in
>> >> > > application settings.
>> >> > >
>> >> > > -Matej
>> >> > >
>> >> > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> >> > > > Frédéric Bertin a écrit :
>> >> > > > > Martijn Dashorst wrote:
>> >> > > > >>> it seems taht this kind of construction is used to make
>> >> workaround
>> >> > > of
>> >> > > > >>> the bug. Is'n it?
>> >> > > > >>
>> >> > > > >> First, what bug? I don't know that this is a bug? I 
>> thought we
>> >> are
>> >> > > > >> discussing a feature here. Secondly, this is not a 
>> workaround,
>> >> but
>> >> > > > >> creating client side code based on a API contract:
>> >> setVisible(false)
>> >> > > > >> removes the component markup completely, including its tags,
>> >> from
>> >> the
>> >> > > > >> final markup.
>> >> > > > > ok, then there's a need for 2 different mechanisms.
>> >> > > > > One to switch a component visibility. This one should be
>> >> *reversible*
>> >> > > > > (in ajax or not), and then it should always output a tag with
>> an
>> >> id
>> >> > > > > attribute (actually, can be done only when 
>> setOutputMarkupId is
>> >> set to
>> >> > > > > true).
>> >> > > > >
>> >> > > > > Another to render or not a component in the output markup. 
>> This
>> >> one
>> >> > > > > should be documented as *not reversible*. This is the current
>> >> > > > > setVisible implementation.
>> >> > > > >
>> >> > > > > wdyt?
>> >> > > >
>> >> > > > +1
>> >> > > > What about keeping current behavior on setVisible (deprecated)
>> and
>> >> > > > add a method setVisibility :
>> >> > > >     - none : render nothing
>> >> > > >     - visible : render all
>> >> > > >     - invisible : render only container tag with
>> >> style:display-none
>> >> > > >
>> >> > > > Add in documentation
>> >> > > > none: can not become visible in ajax
>> >> > > > in visible: can
>> >> > > >
>> >> > > > I think it will match to all use case, no ?
>> >> > > > >
>> >> > > > >
>> >> > > > >
>> >> > > > >>
>> >> > > > >> It is based on the assumption that some element is *NOT*
>> >> present
>> >> at
>> >> > > > >> all. Your change will invert that behavior, and in such a 
>> way
>> >> that it
>> >> > > > >> is only detectable by debugging your javascript. Not
>> >> something I
>> >> > > > >> enjoy, nor 95% of the development community.
>> >> > > > >>
>> >> > > > >> You must understand that this is a major api break, not
>> >> something
>> >> > > > >> minor. This is not detectable by a compiler. You *will* 
>> break
>> >> > > existing
>> >> > > > >> scripts, pages and applications in a non-obvious way. Silent
>> >> failures
>> >> > > > >> are something we try to avoid at all cost.
>> >> > > > >>
>> >> > > > >> Martijn
>> >> > > > >>
>> >> > > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
>> >> wrote:
>> >> > > > >>> Martijn Dashorst a écrit :
>> >> > > > >>> > Currently everybody assumes (correctly) that the 
>> element is
>> >> > > > >>> completely
>> >> > > > >>> > removed (Ajax and non-Ajax)
>> >> > > > >>> Yes of course, but it is the same for all workarounds ;)
>> >> > > > >>> When to change servlet to filter, users have to change
>> >> their web
>> >> > > xml.
>> >> > > > >>> Each time you change something users have to adapt their 
>> code
>> >> > > > >>> > i.e. not present in the final markup.
>> >> > > > >>> > This means that scripts that iterate through the dom, or
>> >> check
>> >> for
>> >> > > > >>> the
>> >> > > > >>> > document.getElementById() == null will fail if we 
>> implement
>> >> this.
>> >> > > > >>> >
>> >> > > > >>> it seems taht this kind of construction is used to make
>> >> workaround
>> >> > > of
>> >> > > > >>> the bug. Is'n it?
>> >> > > > >>> > I *strongly* discourage changing this behavior.
>> >> > > > >>> >
>> >> > > > >>> > Martijn
>> >> > > > >>> >
>> >> > > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >> > > > >>> >> Will it? This seems to be actually quite a smart
>> >> workaround.
>> >> How
>> >> > > > >>> >> exactly will this break existing clients? Only thing i'm
>> >> > > concerned
>> >> > > > >>> >> about is the validity of output markup. but imho when we
>> >> preserve
>> >> > > > >>> the
>> >> > > > >>> >> original tag names, e.g. td will render as td, it should
>> be
>> >> all
>> >> > > > >>> right.
>> >> > > > >>> >>
>> >> > > > >>> >> -Matej
>> >> > > > >>> >>
>> >> > > > >>> >> On 3/19/07, Martijn Dashorst 
>> <ma...@gmail.com>
>> >> wrote:
>> >> > > > >>> >> > So you mean:
>> >> > > > >>> >> >
>> >> > > > >>> >> >     Label l = Label("foo", "hello");
>> >> > > > >>> >> > renders:
>> >> > > > >>> >> >     <span wicket:id="foo">hello</span>
>> >> > > > >>> >> >
>> >> > > > >>> >> > ... some ajax stuff, or a normal page render:
>> >> > > > >>> >> >
>> >> > > > >>> >> >     l.setVisible(false);
>> >> > > > >>> >> > renders:
>> >> > > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
>> >> > > > >>> >> > ?
>> >> > > > >>> >> >
>> >> > > > >>> >> > This can and will break existing clients in a very 
>> nasty
>> >> > > manner,
>> >> > > > >>> >> > because the markup id is still present in the final
>> >> markup.
>> >> > > > >>> >> >
>> >> > > > >>> >> > Martijn
>> >> > > > >>> >> >
>> >> > > > >>> >> > On 3/19/07, Vincent Demay
>> >> <vi...@anyware-tech.com>
>> >> > > wrote:
>> >> > > > >>> >> > > Johan Compagner a écrit :
>> >> > > > >>> >> > > >> > Also always just rendering the component but
>> >> use the
>> >> > > style
>> >> > > > >>> >> to make in
>> >> > > > >>> >> > > >> > invisible
>> >> > > > >>> >> > > >> > could be a security problem. So that can't 
>> be the
>> >> > > default.
>> >> > > > >>> >> > > >>
>> >> > > > >>> >> > > >> What do you mean by security problem?
>> >> > > > >>> >> > > >
>> >> > > > >>> >> > > >
>> >> > > > >>> >> > > >
>> >> > > > >>> >> > > > If the the component that is  set to none 
>> visible is
>> >> none
>> >> > > > >>> visible
>> >> > > > >>> >> > > > because of
>> >> > > > >>> >> > > > security
>> >> > > > >>> >> > > > So it has data that never should be send to the
>> >> browser
>> >> > > > >>> because
>> >> > > > >>> >> the
>> >> > > > >>> >> > > > user is
>> >> > > > >>> >> > > > not allowed
>> >> > > > >>> >> > > > to see it.
>> >> > > > >>> >> > >
>> >> > > > >>> >> > > But data is never send to the user because a none
>> >> visible
>> >> > > > >>> >> component will
>> >> > > > >>> >> > > be render as an empty tag, so without data
>> >> > > > >>> >> > >
>> >> > > > >>> >> > >
>> >> > > > >>> >> >
>> >> > > > >>> >> >
>> >> > > > >>> >> > --
>> >> > > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> >> > > > >>> >> > Join the wicket community at irc.freenode.net: 
>> ##wicket
>> >> > > > >>> >> > Wicket 1.2.5 will keep your server alive. Download
>> Wicket
>> >> now!
>> >> > > > >>> >> > http://wicketframework.org
>> >> > > > >>> >> >
>> >> > > > >>> >>
>> >> > > > >>> >
>> >> > > > >>> >
>> >> > > > >>>
>> >> > > > >>>
>> >> > > > >>
>> >> > > > >>
>> >> > > > >
>> >> > > > >
>> >> > > >
>> >> > > >
>> >> > >
>> >> >
>> >>
>> >
>>
>>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Igor Vaynberg <ig...@gmail.com>.
On 3/19/07, Frédéric Bertin <fr...@anyware-tech.com> wrote:
>
> However, from a user point of view, I find it a little bit complicated
> and would find really nice to have a higher level method, like
> setAjaxRefresheable(true|false),


we should keep the term "ajax" out of the method name. it needs to be more
abstract.

which takes care of
> tuning all the setOutput* stuff so that a component is *always*
> refresheable in ajax, even if it is initially setVisible(false), or
> anything else.



> As a user, I want to refresh my component with Ajax. I don't care if, to
> achieve that, Wicket needs a tag, an id, or anything else.


which is what i proposed below in my code snippet

-igor



Does it make sense?
>
> Fred
>
> Igor Vaynberg wrote:
> > im not a big fan of having an application-wide setting for this. it will
> > make it harder to integrate with 3rd party components that werent
> written
> > with the setting in mind because it pretty much gives you an option of
> > ignoring it by assuming a default.
> >
> > better make it explicit, and it doesnt add any extra work. instead of
> > calling setoutputmarkupid(true) just call setoutputmarkuptag() or
> > maybe even
> > call it setoutputmarkuptagandid()
> >
> > -igor
> >
> >
> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >>
> >> Well, I'm also -1 for having this behavior off for default.
> >> But, i think it can be quite useful for couple of cases, therefore i
> >> think there should be support for that.
> >>
> >> I think what we need is an application setting, for default behavior,
> >> and then a getter for each component.
> >> e.g. boolean Component.getRenderEmptyTagOnNotVisible()
> >> that would by default return the application settings value.
> >>
> >> This would cost us nothing, as there is not even flag needed for it.
> >> And would be versatile enough, to only enable the behavior on
> >> components where it really make sense and doesn't break anything. But
> >> this doesn't mean i couldn't live with a component flag of course :)
> >>
> >> -Matej
> >>
> >> On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
> >> > what you need is a different method. instead of
> >> setoutputmarkupid(true)
> >> you
> >> > need something that will do
> >> >
> >> > setoutputmarkuptag(boolean b) {
> >> >   if (b) {
> >> >      setoutputmarkupid(true);
> >> >      setflag(outputmarkuptag,true);
> >> >    } else {
> >> >        setflag(outputmarkuptag,false);
> >> >        //not sure if we should undo setoutputmarkupid here
> >> >     }
> >> > }
> >> >
> >> > now this will solve the problems outlined so far
> >> >
> >> > btw the latest patch attached to the issue is bad because it executes
> >> > behaviors which should not happen.
> >> >
> >> > what i am worried about are javascript libraries and css. does css 3
> >> have
> >> > support for odd/even selectors? that will break. this will even break
> >> css 2
> >> > first child selector if that happens to be invisible because i dont
> >> think it
> >> > checks for the display attribute. it will also break  3rd party
> >> javascript
> >> > libs that dont check for display attrs.
> >> >
> >> > i see how this makes life easier, but it also has a potential to be
> >> evil.
> >> > that is why i closed the issue as wont-fix and told vincent to
> >> bring the
> >> > discussion here on the list before we go any further.
> >> >
> >> > -igor
> >> >
> >> >
> >> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> > >
> >> > > I don't like it. I don't think deprecating set/isVisible is the
> >> way to
> >> > > go. Plus there are other reasons: As now we use one flag for
> visible
> >> > > status. With your approach you'd require an enum, int, byte or
> >> > > something similiar, that takes more space then just one bit in
> >> flags.
> >> > >
> >> > > I think we can just make a getter, that by default returns the
> value
> >> > > from application settings. And you can override that for your
> >> > > component, if you want something different that what's set in
> >> > > application settings.
> >> > >
> >> > > -Matej
> >> > >
> >> > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >> > > > Frédéric Bertin a écrit :
> >> > > > > Martijn Dashorst wrote:
> >> > > > >>> it seems taht this kind of construction is used to make
> >> workaround
> >> > > of
> >> > > > >>> the bug. Is'n it?
> >> > > > >>
> >> > > > >> First, what bug? I don't know that this is a bug? I thought we
> >> are
> >> > > > >> discussing a feature here. Secondly, this is not a workaround,
> >> but
> >> > > > >> creating client side code based on a API contract:
> >> setVisible(false)
> >> > > > >> removes the component markup completely, including its tags,
> >> from
> >> the
> >> > > > >> final markup.
> >> > > > > ok, then there's a need for 2 different mechanisms.
> >> > > > > One to switch a component visibility. This one should be
> >> *reversible*
> >> > > > > (in ajax or not), and then it should always output a tag with
> an
> >> id
> >> > > > > attribute (actually, can be done only when setOutputMarkupId is
> >> set to
> >> > > > > true).
> >> > > > >
> >> > > > > Another to render or not a component in the output markup. This
> >> one
> >> > > > > should be documented as *not reversible*. This is the current
> >> > > > > setVisible implementation.
> >> > > > >
> >> > > > > wdyt?
> >> > > >
> >> > > > +1
> >> > > > What about keeping current behavior on setVisible (deprecated)
> and
> >> > > > add a method setVisibility :
> >> > > >     - none : render nothing
> >> > > >     - visible : render all
> >> > > >     - invisible : render only container tag with
> >> style:display-none
> >> > > >
> >> > > > Add in documentation
> >> > > > none: can not become visible in ajax
> >> > > > in visible: can
> >> > > >
> >> > > > I think it will match to all use case, no ?
> >> > > > >
> >> > > > >
> >> > > > >
> >> > > > >>
> >> > > > >> It is based on the assumption that some element is *NOT*
> >> present
> >> at
> >> > > > >> all. Your change will invert that behavior, and in such a way
> >> that it
> >> > > > >> is only detectable by debugging your javascript. Not
> >> something I
> >> > > > >> enjoy, nor 95% of the development community.
> >> > > > >>
> >> > > > >> You must understand that this is a major api break, not
> >> something
> >> > > > >> minor. This is not detectable by a compiler. You *will* break
> >> > > existing
> >> > > > >> scripts, pages and applications in a non-obvious way. Silent
> >> failures
> >> > > > >> are something we try to avoid at all cost.
> >> > > > >>
> >> > > > >> Martijn
> >> > > > >>
> >> > > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> >> wrote:
> >> > > > >>> Martijn Dashorst a écrit :
> >> > > > >>> > Currently everybody assumes (correctly) that the element is
> >> > > > >>> completely
> >> > > > >>> > removed (Ajax and non-Ajax)
> >> > > > >>> Yes of course, but it is the same for all workarounds ;)
> >> > > > >>> When to change servlet to filter, users have to change
> >> their web
> >> > > xml.
> >> > > > >>> Each time you change something users have to adapt their code
> >> > > > >>> > i.e. not present in the final markup.
> >> > > > >>> > This means that scripts that iterate through the dom, or
> >> check
> >> for
> >> > > > >>> the
> >> > > > >>> > document.getElementById() == null will fail if we implement
> >> this.
> >> > > > >>> >
> >> > > > >>> it seems taht this kind of construction is used to make
> >> workaround
> >> > > of
> >> > > > >>> the bug. Is'n it?
> >> > > > >>> > I *strongly* discourage changing this behavior.
> >> > > > >>> >
> >> > > > >>> > Martijn
> >> > > > >>> >
> >> > > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> > > > >>> >> Will it? This seems to be actually quite a smart
> >> workaround.
> >> How
> >> > > > >>> >> exactly will this break existing clients? Only thing i'm
> >> > > concerned
> >> > > > >>> >> about is the validity of output markup. but imho when we
> >> preserve
> >> > > > >>> the
> >> > > > >>> >> original tag names, e.g. td will render as td, it should
> be
> >> all
> >> > > > >>> right.
> >> > > > >>> >>
> >> > > > >>> >> -Matej
> >> > > > >>> >>
> >> > > > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
> >> wrote:
> >> > > > >>> >> > So you mean:
> >> > > > >>> >> >
> >> > > > >>> >> >     Label l = Label("foo", "hello");
> >> > > > >>> >> > renders:
> >> > > > >>> >> >     <span wicket:id="foo">hello</span>
> >> > > > >>> >> >
> >> > > > >>> >> > ... some ajax stuff, or a normal page render:
> >> > > > >>> >> >
> >> > > > >>> >> >     l.setVisible(false);
> >> > > > >>> >> > renders:
> >> > > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> >> > > > >>> >> > ?
> >> > > > >>> >> >
> >> > > > >>> >> > This can and will break existing clients in a very nasty
> >> > > manner,
> >> > > > >>> >> > because the markup id is still present in the final
> >> markup.
> >> > > > >>> >> >
> >> > > > >>> >> > Martijn
> >> > > > >>> >> >
> >> > > > >>> >> > On 3/19/07, Vincent Demay
> >> <vi...@anyware-tech.com>
> >> > > wrote:
> >> > > > >>> >> > > Johan Compagner a écrit :
> >> > > > >>> >> > > >> > Also always just rendering the component but
> >> use the
> >> > > style
> >> > > > >>> >> to make in
> >> > > > >>> >> > > >> > invisible
> >> > > > >>> >> > > >> > could be a security problem. So that can't be the
> >> > > default.
> >> > > > >>> >> > > >>
> >> > > > >>> >> > > >> What do you mean by security problem?
> >> > > > >>> >> > > >
> >> > > > >>> >> > > >
> >> > > > >>> >> > > >
> >> > > > >>> >> > > > If the the component that is  set to none visible is
> >> none
> >> > > > >>> visible
> >> > > > >>> >> > > > because of
> >> > > > >>> >> > > > security
> >> > > > >>> >> > > > So it has data that never should be send to the
> >> browser
> >> > > > >>> because
> >> > > > >>> >> the
> >> > > > >>> >> > > > user is
> >> > > > >>> >> > > > not allowed
> >> > > > >>> >> > > > to see it.
> >> > > > >>> >> > >
> >> > > > >>> >> > > But data is never send to the user because a none
> >> visible
> >> > > > >>> >> component will
> >> > > > >>> >> > > be render as an empty tag, so without data
> >> > > > >>> >> > >
> >> > > > >>> >> > >
> >> > > > >>> >> >
> >> > > > >>> >> >
> >> > > > >>> >> > --
> >> > > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> >> > > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> >> > > > >>> >> > Wicket 1.2.5 will keep your server alive. Download
> Wicket
> >> now!
> >> > > > >>> >> > http://wicketframework.org
> >> > > > >>> >> >
> >> > > > >>> >>
> >> > > > >>> >
> >> > > > >>> >
> >> > > > >>>
> >> > > > >>>
> >> > > > >>
> >> > > > >>
> >> > > > >
> >> > > > >
> >> > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Frédéric Bertin <fr...@anyware-tech.com>.
ok, let's go for it.

However, from a user point of view, I find it a little bit complicated 
and would find really nice to have a higher level method, like 
setAjaxRefresheable(true|false), which takes care of
tuning all the setOutput* stuff so that a component is *always* 
refresheable in ajax, even if it is initially setVisible(false), or 
anything else.
As a user, I want to refresh my component with Ajax. I don't care if, to 
achieve that, Wicket needs a tag, an id, or anything else.

Does it make sense?

Fred

Igor Vaynberg wrote:
> im not a big fan of having an application-wide setting for this. it will
> make it harder to integrate with 3rd party components that werent written
> with the setting in mind because it pretty much gives you an option of
> ignoring it by assuming a default.
>
> better make it explicit, and it doesnt add any extra work. instead of
> calling setoutputmarkupid(true) just call setoutputmarkuptag() or 
> maybe even
> call it setoutputmarkuptagandid()
>
> -igor
>
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>>
>> Well, I'm also -1 for having this behavior off for default.
>> But, i think it can be quite useful for couple of cases, therefore i
>> think there should be support for that.
>>
>> I think what we need is an application setting, for default behavior,
>> and then a getter for each component.
>> e.g. boolean Component.getRenderEmptyTagOnNotVisible()
>> that would by default return the application settings value.
>>
>> This would cost us nothing, as there is not even flag needed for it.
>> And would be versatile enough, to only enable the behavior on
>> components where it really make sense and doesn't break anything. But
>> this doesn't mean i couldn't live with a component flag of course :)
>>
>> -Matej
>>
>> On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
>> > what you need is a different method. instead of 
>> setoutputmarkupid(true)
>> you
>> > need something that will do
>> >
>> > setoutputmarkuptag(boolean b) {
>> >   if (b) {
>> >      setoutputmarkupid(true);
>> >      setflag(outputmarkuptag,true);
>> >    } else {
>> >        setflag(outputmarkuptag,false);
>> >        //not sure if we should undo setoutputmarkupid here
>> >     }
>> > }
>> >
>> > now this will solve the problems outlined so far
>> >
>> > btw the latest patch attached to the issue is bad because it executes
>> > behaviors which should not happen.
>> >
>> > what i am worried about are javascript libraries and css. does css 3
>> have
>> > support for odd/even selectors? that will break. this will even break
>> css 2
>> > first child selector if that happens to be invisible because i dont
>> think it
>> > checks for the display attribute. it will also break  3rd party
>> javascript
>> > libs that dont check for display attrs.
>> >
>> > i see how this makes life easier, but it also has a potential to be
>> evil.
>> > that is why i closed the issue as wont-fix and told vincent to 
>> bring the
>> > discussion here on the list before we go any further.
>> >
>> > -igor
>> >
>> >
>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> > >
>> > > I don't like it. I don't think deprecating set/isVisible is the 
>> way to
>> > > go. Plus there are other reasons: As now we use one flag for visible
>> > > status. With your approach you'd require an enum, int, byte or
>> > > something similiar, that takes more space then just one bit in 
>> flags.
>> > >
>> > > I think we can just make a getter, that by default returns the value
>> > > from application settings. And you can override that for your
>> > > component, if you want something different that what's set in
>> > > application settings.
>> > >
>> > > -Matej
>> > >
>> > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> > > > Frédéric Bertin a écrit :
>> > > > > Martijn Dashorst wrote:
>> > > > >>> it seems taht this kind of construction is used to make
>> workaround
>> > > of
>> > > > >>> the bug. Is'n it?
>> > > > >>
>> > > > >> First, what bug? I don't know that this is a bug? I thought we
>> are
>> > > > >> discussing a feature here. Secondly, this is not a workaround,
>> but
>> > > > >> creating client side code based on a API contract:
>> setVisible(false)
>> > > > >> removes the component markup completely, including its tags, 
>> from
>> the
>> > > > >> final markup.
>> > > > > ok, then there's a need for 2 different mechanisms.
>> > > > > One to switch a component visibility. This one should be
>> *reversible*
>> > > > > (in ajax or not), and then it should always output a tag with an
>> id
>> > > > > attribute (actually, can be done only when setOutputMarkupId is
>> set to
>> > > > > true).
>> > > > >
>> > > > > Another to render or not a component in the output markup. This
>> one
>> > > > > should be documented as *not reversible*. This is the current
>> > > > > setVisible implementation.
>> > > > >
>> > > > > wdyt?
>> > > >
>> > > > +1
>> > > > What about keeping current behavior on setVisible (deprecated) and
>> > > > add a method setVisibility :
>> > > >     - none : render nothing
>> > > >     - visible : render all
>> > > >     - invisible : render only container tag with 
>> style:display-none
>> > > >
>> > > > Add in documentation
>> > > > none: can not become visible in ajax
>> > > > in visible: can
>> > > >
>> > > > I think it will match to all use case, no ?
>> > > > >
>> > > > >
>> > > > >
>> > > > >>
>> > > > >> It is based on the assumption that some element is *NOT* 
>> present
>> at
>> > > > >> all. Your change will invert that behavior, and in such a way
>> that it
>> > > > >> is only detectable by debugging your javascript. Not 
>> something I
>> > > > >> enjoy, nor 95% of the development community.
>> > > > >>
>> > > > >> You must understand that this is a major api break, not 
>> something
>> > > > >> minor. This is not detectable by a compiler. You *will* break
>> > > existing
>> > > > >> scripts, pages and applications in a non-obvious way. Silent
>> failures
>> > > > >> are something we try to avoid at all cost.
>> > > > >>
>> > > > >> Martijn
>> > > > >>
>> > > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> 
>> wrote:
>> > > > >>> Martijn Dashorst a écrit :
>> > > > >>> > Currently everybody assumes (correctly) that the element is
>> > > > >>> completely
>> > > > >>> > removed (Ajax and non-Ajax)
>> > > > >>> Yes of course, but it is the same for all workarounds ;)
>> > > > >>> When to change servlet to filter, users have to change 
>> their web
>> > > xml.
>> > > > >>> Each time you change something users have to adapt their code
>> > > > >>> > i.e. not present in the final markup.
>> > > > >>> > This means that scripts that iterate through the dom, or 
>> check
>> for
>> > > > >>> the
>> > > > >>> > document.getElementById() == null will fail if we implement
>> this.
>> > > > >>> >
>> > > > >>> it seems taht this kind of construction is used to make
>> workaround
>> > > of
>> > > > >>> the bug. Is'n it?
>> > > > >>> > I *strongly* discourage changing this behavior.
>> > > > >>> >
>> > > > >>> > Martijn
>> > > > >>> >
>> > > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> > > > >>> >> Will it? This seems to be actually quite a smart 
>> workaround.
>> How
>> > > > >>> >> exactly will this break existing clients? Only thing i'm
>> > > concerned
>> > > > >>> >> about is the validity of output markup. but imho when we
>> preserve
>> > > > >>> the
>> > > > >>> >> original tag names, e.g. td will render as td, it should be
>> all
>> > > > >>> right.
>> > > > >>> >>
>> > > > >>> >> -Matej
>> > > > >>> >>
>> > > > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
>> wrote:
>> > > > >>> >> > So you mean:
>> > > > >>> >> >
>> > > > >>> >> >     Label l = Label("foo", "hello");
>> > > > >>> >> > renders:
>> > > > >>> >> >     <span wicket:id="foo">hello</span>
>> > > > >>> >> >
>> > > > >>> >> > ... some ajax stuff, or a normal page render:
>> > > > >>> >> >
>> > > > >>> >> >     l.setVisible(false);
>> > > > >>> >> > renders:
>> > > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
>> > > > >>> >> > ?
>> > > > >>> >> >
>> > > > >>> >> > This can and will break existing clients in a very nasty
>> > > manner,
>> > > > >>> >> > because the markup id is still present in the final 
>> markup.
>> > > > >>> >> >
>> > > > >>> >> > Martijn
>> > > > >>> >> >
>> > > > >>> >> > On 3/19/07, Vincent Demay 
>> <vi...@anyware-tech.com>
>> > > wrote:
>> > > > >>> >> > > Johan Compagner a écrit :
>> > > > >>> >> > > >> > Also always just rendering the component but 
>> use the
>> > > style
>> > > > >>> >> to make in
>> > > > >>> >> > > >> > invisible
>> > > > >>> >> > > >> > could be a security problem. So that can't be the
>> > > default.
>> > > > >>> >> > > >>
>> > > > >>> >> > > >> What do you mean by security problem?
>> > > > >>> >> > > >
>> > > > >>> >> > > >
>> > > > >>> >> > > >
>> > > > >>> >> > > > If the the component that is  set to none visible is
>> none
>> > > > >>> visible
>> > > > >>> >> > > > because of
>> > > > >>> >> > > > security
>> > > > >>> >> > > > So it has data that never should be send to the 
>> browser
>> > > > >>> because
>> > > > >>> >> the
>> > > > >>> >> > > > user is
>> > > > >>> >> > > > not allowed
>> > > > >>> >> > > > to see it.
>> > > > >>> >> > >
>> > > > >>> >> > > But data is never send to the user because a none 
>> visible
>> > > > >>> >> component will
>> > > > >>> >> > > be render as an empty tag, so without data
>> > > > >>> >> > >
>> > > > >>> >> > >
>> > > > >>> >> >
>> > > > >>> >> >
>> > > > >>> >> > --
>> > > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> > > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> > > > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
>> now!
>> > > > >>> >> > http://wicketframework.org
>> > > > >>> >> >
>> > > > >>> >>
>> > > > >>> >
>> > > > >>> >
>> > > > >>>
>> > > > >>>
>> > > > >>
>> > > > >>
>> > > > >
>> > > > >
>> > > >
>> > > >
>> > >
>> >
>>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Igor Vaynberg <ig...@gmail.com>.
im not a big fan of having an application-wide setting for this. it will
make it harder to integrate with 3rd party components that werent written
with the setting in mind because it pretty much gives you an option of
ignoring it by assuming a default.

better make it explicit, and it doesnt add any extra work. instead of
calling setoutputmarkupid(true) just call setoutputmarkuptag() or maybe even
call it setoutputmarkuptagandid()

-igor


On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>
> Well, I'm also -1 for having this behavior off for default.
> But, i think it can be quite useful for couple of cases, therefore i
> think there should be support for that.
>
> I think what we need is an application setting, for default behavior,
> and then a getter for each component.
> e.g. boolean Component.getRenderEmptyTagOnNotVisible()
> that would by default return the application settings value.
>
> This would cost us nothing, as there is not even flag needed for it.
> And would be versatile enough, to only enable the behavior on
> components where it really make sense and doesn't break anything. But
> this doesn't mean i couldn't live with a component flag of course :)
>
> -Matej
>
> On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > what you need is a different method. instead of setoutputmarkupid(true)
> you
> > need something that will do
> >
> > setoutputmarkuptag(boolean b) {
> >   if (b) {
> >      setoutputmarkupid(true);
> >      setflag(outputmarkuptag,true);
> >    } else {
> >        setflag(outputmarkuptag,false);
> >        //not sure if we should undo setoutputmarkupid here
> >     }
> > }
> >
> > now this will solve the problems outlined so far
> >
> > btw the latest patch attached to the issue is bad because it executes
> > behaviors which should not happen.
> >
> > what i am worried about are javascript libraries and css. does css 3
> have
> > support for odd/even selectors? that will break. this will even break
> css 2
> > first child selector if that happens to be invisible because i dont
> think it
> > checks for the display attribute. it will also break  3rd party
> javascript
> > libs that dont check for display attrs.
> >
> > i see how this makes life easier, but it also has a potential to be
> evil.
> > that is why i closed the issue as wont-fix and told vincent to bring the
> > discussion here on the list before we go any further.
> >
> > -igor
> >
> >
> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > >
> > > I don't like it. I don't think deprecating set/isVisible is the way to
> > > go. Plus there are other reasons: As now we use one flag for visible
> > > status. With your approach you'd require an enum, int, byte or
> > > something similiar, that takes more space then just one bit in flags.
> > >
> > > I think we can just make a getter, that by default returns the value
> > > from application settings. And you can override that for your
> > > component, if you want something different that what's set in
> > > application settings.
> > >
> > > -Matej
> > >
> > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > > Frédéric Bertin a écrit :
> > > > > Martijn Dashorst wrote:
> > > > >>> it seems taht this kind of construction is used to make
> workaround
> > > of
> > > > >>> the bug. Is'n it?
> > > > >>
> > > > >> First, what bug? I don't know that this is a bug? I thought we
> are
> > > > >> discussing a feature here. Secondly, this is not a workaround,
> but
> > > > >> creating client side code based on a API contract:
> setVisible(false)
> > > > >> removes the component markup completely, including its tags, from
> the
> > > > >> final markup.
> > > > > ok, then there's a need for 2 different mechanisms.
> > > > > One to switch a component visibility. This one should be
> *reversible*
> > > > > (in ajax or not), and then it should always output a tag with an
> id
> > > > > attribute (actually, can be done only when setOutputMarkupId is
> set to
> > > > > true).
> > > > >
> > > > > Another to render or not a component in the output markup. This
> one
> > > > > should be documented as *not reversible*. This is the current
> > > > > setVisible implementation.
> > > > >
> > > > > wdyt?
> > > >
> > > > +1
> > > > What about keeping current behavior on setVisible (deprecated) and
> > > > add a method setVisibility :
> > > >     - none : render nothing
> > > >     - visible : render all
> > > >     - invisible : render only container tag with style:display-none
> > > >
> > > > Add in documentation
> > > > none: can not become visible in ajax
> > > > in visible: can
> > > >
> > > > I think it will match to all use case, no ?
> > > > >
> > > > >
> > > > >
> > > > >>
> > > > >> It is based on the assumption that some element is *NOT* present
> at
> > > > >> all. Your change will invert that behavior, and in such a way
> that it
> > > > >> is only detectable by debugging your javascript. Not something I
> > > > >> enjoy, nor 95% of the development community.
> > > > >>
> > > > >> You must understand that this is a major api break, not something
> > > > >> minor. This is not detectable by a compiler. You *will* break
> > > existing
> > > > >> scripts, pages and applications in a non-obvious way. Silent
> failures
> > > > >> are something we try to avoid at all cost.
> > > > >>
> > > > >> Martijn
> > > > >>
> > > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > > >>> Martijn Dashorst a écrit :
> > > > >>> > Currently everybody assumes (correctly) that the element is
> > > > >>> completely
> > > > >>> > removed (Ajax and non-Ajax)
> > > > >>> Yes of course, but it is the same for all workarounds ;)
> > > > >>> When to change servlet to filter, users have to change their web
> > > xml.
> > > > >>> Each time you change something users have to adapt their code
> > > > >>> > i.e. not present in the final markup.
> > > > >>> > This means that scripts that iterate through the dom, or check
> for
> > > > >>> the
> > > > >>> > document.getElementById() == null will fail if we implement
> this.
> > > > >>> >
> > > > >>> it seems taht this kind of construction is used to make
> workaround
> > > of
> > > > >>> the bug. Is'n it?
> > > > >>> > I *strongly* discourage changing this behavior.
> > > > >>> >
> > > > >>> > Martijn
> > > > >>> >
> > > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > > > >>> >> Will it? This seems to be actually quite a smart workaround.
> How
> > > > >>> >> exactly will this break existing clients? Only thing i'm
> > > concerned
> > > > >>> >> about is the validity of output markup. but imho when we
> preserve
> > > > >>> the
> > > > >>> >> original tag names, e.g. td will render as td, it should be
> all
> > > > >>> right.
> > > > >>> >>
> > > > >>> >> -Matej
> > > > >>> >>
> > > > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
> wrote:
> > > > >>> >> > So you mean:
> > > > >>> >> >
> > > > >>> >> >     Label l = Label("foo", "hello");
> > > > >>> >> > renders:
> > > > >>> >> >     <span wicket:id="foo">hello</span>
> > > > >>> >> >
> > > > >>> >> > ... some ajax stuff, or a normal page render:
> > > > >>> >> >
> > > > >>> >> >     l.setVisible(false);
> > > > >>> >> > renders:
> > > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> > > > >>> >> > ?
> > > > >>> >> >
> > > > >>> >> > This can and will break existing clients in a very nasty
> > > manner,
> > > > >>> >> > because the markup id is still present in the final markup.
> > > > >>> >> >
> > > > >>> >> > Martijn
> > > > >>> >> >
> > > > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> > > wrote:
> > > > >>> >> > > Johan Compagner a écrit :
> > > > >>> >> > > >> > Also always just rendering the component but use the
> > > style
> > > > >>> >> to make in
> > > > >>> >> > > >> > invisible
> > > > >>> >> > > >> > could be a security problem. So that can't be the
> > > default.
> > > > >>> >> > > >>
> > > > >>> >> > > >> What do you mean by security problem?
> > > > >>> >> > > >
> > > > >>> >> > > >
> > > > >>> >> > > >
> > > > >>> >> > > > If the the component that is  set to none visible is
> none
> > > > >>> visible
> > > > >>> >> > > > because of
> > > > >>> >> > > > security
> > > > >>> >> > > > So it has data that never should be send to the browser
> > > > >>> because
> > > > >>> >> the
> > > > >>> >> > > > user is
> > > > >>> >> > > > not allowed
> > > > >>> >> > > > to see it.
> > > > >>> >> > >
> > > > >>> >> > > But data is never send to the user because a none visible
> > > > >>> >> component will
> > > > >>> >> > > be render as an empty tag, so without data
> > > > >>> >> > >
> > > > >>> >> > >
> > > > >>> >> >
> > > > >>> >> >
> > > > >>> >> > --
> > > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> > > > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
> now!
> > > > >>> >> > http://wicketframework.org
> > > > >>> >> >
> > > > >>> >>
> > > > >>> >
> > > > >>> >
> > > > >>>
> > > > >>>
> > > > >>
> > > > >>
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
I wanted to say "on having this behavior _on_ by default" of course :)

On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> Well, I'm also -1 for having this behavior off for default.
> But, i think it can be quite useful for couple of cases, therefore i
> think there should be support for that.
>
> I think what we need is an application setting, for default behavior,
> and then a getter for each component.
> e.g. boolean Component.getRenderEmptyTagOnNotVisible()
> that would by default return the application settings value.
>
> This would cost us nothing, as there is not even flag needed for it.
> And would be versatile enough, to only enable the behavior on
> components where it really make sense and doesn't break anything. But
> this doesn't mean i couldn't live with a component flag of course :)
>
> -Matej
>
> On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > what you need is a different method. instead of setoutputmarkupid(true) you
> > need something that will do
> >
> > setoutputmarkuptag(boolean b) {
> >   if (b) {
> >      setoutputmarkupid(true);
> >      setflag(outputmarkuptag,true);
> >    } else {
> >        setflag(outputmarkuptag,false);
> >        //not sure if we should undo setoutputmarkupid here
> >     }
> > }
> >
> > now this will solve the problems outlined so far
> >
> > btw the latest patch attached to the issue is bad because it executes
> > behaviors which should not happen.
> >
> > what i am worried about are javascript libraries and css. does css 3 have
> > support for odd/even selectors? that will break. this will even break css 2
> > first child selector if that happens to be invisible because i dont think it
> > checks for the display attribute. it will also break  3rd party javascript
> > libs that dont check for display attrs.
> >
> > i see how this makes life easier, but it also has a potential to be evil.
> > that is why i closed the issue as wont-fix and told vincent to bring the
> > discussion here on the list before we go any further.
> >
> > -igor
> >
> >
> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > >
> > > I don't like it. I don't think deprecating set/isVisible is the way to
> > > go. Plus there are other reasons: As now we use one flag for visible
> > > status. With your approach you'd require an enum, int, byte or
> > > something similiar, that takes more space then just one bit in flags.
> > >
> > > I think we can just make a getter, that by default returns the value
> > > from application settings. And you can override that for your
> > > component, if you want something different that what's set in
> > > application settings.
> > >
> > > -Matej
> > >
> > > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > > Frédéric Bertin a écrit :
> > > > > Martijn Dashorst wrote:
> > > > >>> it seems taht this kind of construction is used to make workaround
> > > of
> > > > >>> the bug. Is'n it?
> > > > >>
> > > > >> First, what bug? I don't know that this is a bug? I thought we are
> > > > >> discussing a feature here. Secondly, this is not a workaround, but
> > > > >> creating client side code based on a API contract: setVisible(false)
> > > > >> removes the component markup completely, including its tags, from the
> > > > >> final markup.
> > > > > ok, then there's a need for 2 different mechanisms.
> > > > > One to switch a component visibility. This one should be *reversible*
> > > > > (in ajax or not), and then it should always output a tag with an id
> > > > > attribute (actually, can be done only when setOutputMarkupId is set to
> > > > > true).
> > > > >
> > > > > Another to render or not a component in the output markup. This one
> > > > > should be documented as *not reversible*. This is the current
> > > > > setVisible implementation.
> > > > >
> > > > > wdyt?
> > > >
> > > > +1
> > > > What about keeping current behavior on setVisible (deprecated) and
> > > > add a method setVisibility :
> > > >     - none : render nothing
> > > >     - visible : render all
> > > >     - invisible : render only container tag with style:display-none
> > > >
> > > > Add in documentation
> > > > none: can not become visible in ajax
> > > > in visible: can
> > > >
> > > > I think it will match to all use case, no ?
> > > > >
> > > > >
> > > > >
> > > > >>
> > > > >> It is based on the assumption that some element is *NOT* present at
> > > > >> all. Your change will invert that behavior, and in such a way that it
> > > > >> is only detectable by debugging your javascript. Not something I
> > > > >> enjoy, nor 95% of the development community.
> > > > >>
> > > > >> You must understand that this is a major api break, not something
> > > > >> minor. This is not detectable by a compiler. You *will* break
> > > existing
> > > > >> scripts, pages and applications in a non-obvious way. Silent failures
> > > > >> are something we try to avoid at all cost.
> > > > >>
> > > > >> Martijn
> > > > >>
> > > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > > >>> Martijn Dashorst a écrit :
> > > > >>> > Currently everybody assumes (correctly) that the element is
> > > > >>> completely
> > > > >>> > removed (Ajax and non-Ajax)
> > > > >>> Yes of course, but it is the same for all workarounds ;)
> > > > >>> When to change servlet to filter, users have to change their web
> > > xml.
> > > > >>> Each time you change something users have to adapt their code
> > > > >>> > i.e. not present in the final markup.
> > > > >>> > This means that scripts that iterate through the dom, or check for
> > > > >>> the
> > > > >>> > document.getElementById() == null will fail if we implement this.
> > > > >>> >
> > > > >>> it seems taht this kind of construction is used to make workaround
> > > of
> > > > >>> the bug. Is'n it?
> > > > >>> > I *strongly* discourage changing this behavior.
> > > > >>> >
> > > > >>> > Martijn
> > > > >>> >
> > > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > > > >>> >> Will it? This seems to be actually quite a smart workaround. How
> > > > >>> >> exactly will this break existing clients? Only thing i'm
> > > concerned
> > > > >>> >> about is the validity of output markup. but imho when we preserve
> > > > >>> the
> > > > >>> >> original tag names, e.g. td will render as td, it should be all
> > > > >>> right.
> > > > >>> >>
> > > > >>> >> -Matej
> > > > >>> >>
> > > > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > > >>> >> > So you mean:
> > > > >>> >> >
> > > > >>> >> >     Label l = Label("foo", "hello");
> > > > >>> >> > renders:
> > > > >>> >> >     <span wicket:id="foo">hello</span>
> > > > >>> >> >
> > > > >>> >> > ... some ajax stuff, or a normal page render:
> > > > >>> >> >
> > > > >>> >> >     l.setVisible(false);
> > > > >>> >> > renders:
> > > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> > > > >>> >> > ?
> > > > >>> >> >
> > > > >>> >> > This can and will break existing clients in a very nasty
> > > manner,
> > > > >>> >> > because the markup id is still present in the final markup.
> > > > >>> >> >
> > > > >>> >> > Martijn
> > > > >>> >> >
> > > > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> > > wrote:
> > > > >>> >> > > Johan Compagner a écrit :
> > > > >>> >> > > >> > Also always just rendering the component but use the
> > > style
> > > > >>> >> to make in
> > > > >>> >> > > >> > invisible
> > > > >>> >> > > >> > could be a security problem. So that can't be the
> > > default.
> > > > >>> >> > > >>
> > > > >>> >> > > >> What do you mean by security problem?
> > > > >>> >> > > >
> > > > >>> >> > > >
> > > > >>> >> > > >
> > > > >>> >> > > > If the the component that is  set to none visible is none
> > > > >>> visible
> > > > >>> >> > > > because of
> > > > >>> >> > > > security
> > > > >>> >> > > > So it has data that never should be send to the browser
> > > > >>> because
> > > > >>> >> the
> > > > >>> >> > > > user is
> > > > >>> >> > > > not allowed
> > > > >>> >> > > > to see it.
> > > > >>> >> > >
> > > > >>> >> > > But data is never send to the user because a none visible
> > > > >>> >> component will
> > > > >>> >> > > be render as an empty tag, so without data
> > > > >>> >> > >
> > > > >>> >> > >
> > > > >>> >> >
> > > > >>> >> >
> > > > >>> >> > --
> > > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> > > > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > > > >>> >> > http://wicketframework.org
> > > > >>> >> >
> > > > >>> >>
> > > > >>> >
> > > > >>> >
> > > > >>>
> > > > >>>
> > > > >>
> > > > >>
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
Well, I'm also -1 for having this behavior off for default.
But, i think it can be quite useful for couple of cases, therefore i
think there should be support for that.

I think what we need is an application setting, for default behavior,
and then a getter for each component.
e.g. boolean Component.getRenderEmptyTagOnNotVisible()
that would by default return the application settings value.

This would cost us nothing, as there is not even flag needed for it.
And would be versatile enough, to only enable the behavior on
components where it really make sense and doesn't break anything. But
this doesn't mean i couldn't live with a component flag of course :)

-Matej

On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
> what you need is a different method. instead of setoutputmarkupid(true) you
> need something that will do
>
> setoutputmarkuptag(boolean b) {
>   if (b) {
>      setoutputmarkupid(true);
>      setflag(outputmarkuptag,true);
>    } else {
>        setflag(outputmarkuptag,false);
>        //not sure if we should undo setoutputmarkupid here
>     }
> }
>
> now this will solve the problems outlined so far
>
> btw the latest patch attached to the issue is bad because it executes
> behaviors which should not happen.
>
> what i am worried about are javascript libraries and css. does css 3 have
> support for odd/even selectors? that will break. this will even break css 2
> first child selector if that happens to be invisible because i dont think it
> checks for the display attribute. it will also break  3rd party javascript
> libs that dont check for display attrs.
>
> i see how this makes life easier, but it also has a potential to be evil.
> that is why i closed the issue as wont-fix and told vincent to bring the
> discussion here on the list before we go any further.
>
> -igor
>
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >
> > I don't like it. I don't think deprecating set/isVisible is the way to
> > go. Plus there are other reasons: As now we use one flag for visible
> > status. With your approach you'd require an enum, int, byte or
> > something similiar, that takes more space then just one bit in flags.
> >
> > I think we can just make a getter, that by default returns the value
> > from application settings. And you can override that for your
> > component, if you want something different that what's set in
> > application settings.
> >
> > -Matej
> >
> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > Frédéric Bertin a écrit :
> > > > Martijn Dashorst wrote:
> > > >>> it seems taht this kind of construction is used to make workaround
> > of
> > > >>> the bug. Is'n it?
> > > >>
> > > >> First, what bug? I don't know that this is a bug? I thought we are
> > > >> discussing a feature here. Secondly, this is not a workaround, but
> > > >> creating client side code based on a API contract: setVisible(false)
> > > >> removes the component markup completely, including its tags, from the
> > > >> final markup.
> > > > ok, then there's a need for 2 different mechanisms.
> > > > One to switch a component visibility. This one should be *reversible*
> > > > (in ajax or not), and then it should always output a tag with an id
> > > > attribute (actually, can be done only when setOutputMarkupId is set to
> > > > true).
> > > >
> > > > Another to render or not a component in the output markup. This one
> > > > should be documented as *not reversible*. This is the current
> > > > setVisible implementation.
> > > >
> > > > wdyt?
> > >
> > > +1
> > > What about keeping current behavior on setVisible (deprecated) and
> > > add a method setVisibility :
> > >     - none : render nothing
> > >     - visible : render all
> > >     - invisible : render only container tag with style:display-none
> > >
> > > Add in documentation
> > > none: can not become visible in ajax
> > > in visible: can
> > >
> > > I think it will match to all use case, no ?
> > > >
> > > >
> > > >
> > > >>
> > > >> It is based on the assumption that some element is *NOT* present at
> > > >> all. Your change will invert that behavior, and in such a way that it
> > > >> is only detectable by debugging your javascript. Not something I
> > > >> enjoy, nor 95% of the development community.
> > > >>
> > > >> You must understand that this is a major api break, not something
> > > >> minor. This is not detectable by a compiler. You *will* break
> > existing
> > > >> scripts, pages and applications in a non-obvious way. Silent failures
> > > >> are something we try to avoid at all cost.
> > > >>
> > > >> Martijn
> > > >>
> > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > >>> Martijn Dashorst a écrit :
> > > >>> > Currently everybody assumes (correctly) that the element is
> > > >>> completely
> > > >>> > removed (Ajax and non-Ajax)
> > > >>> Yes of course, but it is the same for all workarounds ;)
> > > >>> When to change servlet to filter, users have to change their web
> > xml.
> > > >>> Each time you change something users have to adapt their code
> > > >>> > i.e. not present in the final markup.
> > > >>> > This means that scripts that iterate through the dom, or check for
> > > >>> the
> > > >>> > document.getElementById() == null will fail if we implement this.
> > > >>> >
> > > >>> it seems taht this kind of construction is used to make workaround
> > of
> > > >>> the bug. Is'n it?
> > > >>> > I *strongly* discourage changing this behavior.
> > > >>> >
> > > >>> > Martijn
> > > >>> >
> > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > > >>> >> Will it? This seems to be actually quite a smart workaround. How
> > > >>> >> exactly will this break existing clients? Only thing i'm
> > concerned
> > > >>> >> about is the validity of output markup. but imho when we preserve
> > > >>> the
> > > >>> >> original tag names, e.g. td will render as td, it should be all
> > > >>> right.
> > > >>> >>
> > > >>> >> -Matej
> > > >>> >>
> > > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > >>> >> > So you mean:
> > > >>> >> >
> > > >>> >> >     Label l = Label("foo", "hello");
> > > >>> >> > renders:
> > > >>> >> >     <span wicket:id="foo">hello</span>
> > > >>> >> >
> > > >>> >> > ... some ajax stuff, or a normal page render:
> > > >>> >> >
> > > >>> >> >     l.setVisible(false);
> > > >>> >> > renders:
> > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> > > >>> >> > ?
> > > >>> >> >
> > > >>> >> > This can and will break existing clients in a very nasty
> > manner,
> > > >>> >> > because the markup id is still present in the final markup.
> > > >>> >> >
> > > >>> >> > Martijn
> > > >>> >> >
> > > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> > wrote:
> > > >>> >> > > Johan Compagner a écrit :
> > > >>> >> > > >> > Also always just rendering the component but use the
> > style
> > > >>> >> to make in
> > > >>> >> > > >> > invisible
> > > >>> >> > > >> > could be a security problem. So that can't be the
> > default.
> > > >>> >> > > >>
> > > >>> >> > > >> What do you mean by security problem?
> > > >>> >> > > >
> > > >>> >> > > >
> > > >>> >> > > >
> > > >>> >> > > > If the the component that is  set to none visible is none
> > > >>> visible
> > > >>> >> > > > because of
> > > >>> >> > > > security
> > > >>> >> > > > So it has data that never should be send to the browser
> > > >>> because
> > > >>> >> the
> > > >>> >> > > > user is
> > > >>> >> > > > not allowed
> > > >>> >> > > > to see it.
> > > >>> >> > >
> > > >>> >> > > But data is never send to the user because a none visible
> > > >>> >> component will
> > > >>> >> > > be render as an empty tag, so without data
> > > >>> >> > >
> > > >>> >> > >
> > > >>> >> >
> > > >>> >> >
> > > >>> >> > --
> > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> > > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > > >>> >> > http://wicketframework.org
> > > >>> >> >
> > > >>> >>
> > > >>> >
> > > >>> >
> > > >>>
> > > >>>
> > > >>
> > > >>
> > > >
> > > >
> > >
> > >
> >
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Igor Vaynberg <ig...@gmail.com>.
hrm....
this name is still bad

maybe setoutputtagplaceholder() is better

it needs to communicate that the tag is still there even though the
component is invisible

-igor


On 3/19/07, Igor Vaynberg <ig...@gmail.com> wrote:
>
> what you need is a different method. instead of setoutputmarkupid(true)
> you need something that will do
>
> setoutputmarkuptag(boolean b) {
>   if (b) {
>      setoutputmarkupid(true);
>      setflag(outputmarkuptag,true);
>    } else {
>        setflag(outputmarkuptag,false);
>        //not sure if we should undo setoutputmarkupid here
>     }
> }
>
> now this will solve the problems outlined so far
>
> btw the latest patch attached to the issue is bad because it executes
> behaviors which should not happen.
>
> what i am worried about are javascript libraries and css. does css 3 have
> support for odd/even selectors? that will break. this will even break css 2
> first child selector if that happens to be invisible because i dont think it
> checks for the display attribute. it will also break  3rd party javascript
> libs that dont check for display attrs.
>
> i see how this makes life easier, but it also has a potential to be evil.
> that is why i closed the issue as wont-fix and told vincent to bring the
> discussion here on the list before we go any further.
>
> -igor
>
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >
> > I don't like it. I don't think deprecating set/isVisible is the way to
> > go. Plus there are other reasons: As now we use one flag for visible
> > status. With your approach you'd require an enum, int, byte or
> > something similiar, that takes more space then just one bit in flags.
> >
> > I think we can just make a getter, that by default returns the value
> > from application settings. And you can override that for your
> > component, if you want something different that what's set in
> > application settings.
> >
> > -Matej
> >
> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > Frédéric Bertin a écrit :
> > > > Martijn Dashorst wrote:
> > > >>> it seems taht this kind of construction is used to make workaround
> > of
> > > >>> the bug. Is'n it?
> > > >>
> > > >> First, what bug? I don't know that this is a bug? I thought we are
> > > >> discussing a feature here. Secondly, this is not a workaround, but
> > > >> creating client side code based on a API contract:
> > setVisible(false)
> > > >> removes the component markup completely, including its tags, from
> > the
> > > >> final markup.
> > > > ok, then there's a need for 2 different mechanisms.
> > > > One to switch a component visibility. This one should be
> > *reversible*
> > > > (in ajax or not), and then it should always output a tag with an id
> > > > attribute (actually, can be done only when setOutputMarkupId is set
> > to
> > > > true).
> > > >
> > > > Another to render or not a component in the output markup. This one
> > > > should be documented as *not reversible*. This is the current
> > > > setVisible implementation.
> > > >
> > > > wdyt?
> > >
> > > +1
> > > What about keeping current behavior on setVisible (deprecated) and
> > > add a method setVisibility :
> > >     - none : render nothing
> > >     - visible : render all
> > >     - invisible : render only container tag with style:display-none
> > >
> > > Add in documentation
> > > none: can not become visible in ajax
> > > in visible: can
> > >
> > > I think it will match to all use case, no ?
> > > >
> > > >
> > > >
> > > >>
> > > >> It is based on the assumption that some element is *NOT* present at
> > > >> all. Your change will invert that behavior, and in such a way that
> > it
> > > >> is only detectable by debugging your javascript. Not something I
> > > >> enjoy, nor 95% of the development community.
> > > >>
> > > >> You must understand that this is a major api break, not something
> > > >> minor. This is not detectable by a compiler. You *will* break
> > existing
> > > >> scripts, pages and applications in a non-obvious way. Silent
> > failures
> > > >> are something we try to avoid at all cost.
> > > >>
> > > >> Martijn
> > > >>
> > > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > >>> Martijn Dashorst a écrit :
> > > >>> > Currently everybody assumes (correctly) that the element is
> > > >>> completely
> > > >>> > removed (Ajax and non-Ajax)
> > > >>> Yes of course, but it is the same for all workarounds ;)
> > > >>> When to change servlet to filter, users have to change their web
> > xml.
> > > >>> Each time you change something users have to adapt their code
> > > >>> > i.e. not present in the final markup.
> > > >>> > This means that scripts that iterate through the dom, or check
> > for
> > > >>> the
> > > >>> > document.getElementById() == null will fail if we implement
> > this.
> > > >>> >
> > > >>> it seems taht this kind of construction is used to make workaround
> > of
> > > >>> the bug. Is'n it?
> > > >>> > I *strongly* discourage changing this behavior.
> > > >>> >
> > > >>> > Martijn
> > > >>> >
> > > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > > >>> >> Will it? This seems to be actually quite a smart workaround.
> > How
> > > >>> >> exactly will this break existing clients? Only thing i'm
> > concerned
> > > >>> >> about is the validity of output markup. but imho when we
> > preserve
> > > >>> the
> > > >>> >> original tag names, e.g. td will render as td, it should be all
> > > >>> right.
> > > >>> >>
> > > >>> >> -Matej
> > > >>> >>
> > > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
> > wrote:
> > > >>> >> > So you mean:
> > > >>> >> >
> > > >>> >> >     Label l = Label("foo", "hello");
> > > >>> >> > renders:
> > > >>> >> >     <span wicket:id="foo">hello</span>
> > > >>> >> >
> > > >>> >> > ... some ajax stuff, or a normal page render:
> > > >>> >> >
> > > >>> >> >     l.setVisible(false);
> > > >>> >> > renders:
> > > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> > > >>> >> > ?
> > > >>> >> >
> > > >>> >> > This can and will break existing clients in a very nasty
> > manner,
> > > >>> >> > because the markup id is still present in the final markup.
> > > >>> >> >
> > > >>> >> > Martijn
> > > >>> >> >
> > > >>> >> > On 3/19/07, Vincent Demay < vincent.demay@anyware-tech.com>
> > wrote:
> > > >>> >> > > Johan Compagner a écrit :
> > > >>> >> > > >> > Also always just rendering the component but use the
> > style
> > > >>> >> to make in
> > > >>> >> > > >> > invisible
> > > >>> >> > > >> > could be a security problem. So that can't be the
> > default.
> > > >>> >> > > >>
> > > >>> >> > > >> What do you mean by security problem?
> > > >>> >> > > >
> > > >>> >> > > >
> > > >>> >> > > >
> > > >>> >> > > > If the the component that is  set to none visible is none
> > > >>> visible
> > > >>> >> > > > because of
> > > >>> >> > > > security
> > > >>> >> > > > So it has data that never should be send to the browser
> > > >>> because
> > > >>> >> the
> > > >>> >> > > > user is
> > > >>> >> > > > not allowed
> > > >>> >> > > > to see it.
> > > >>> >> > >
> > > >>> >> > > But data is never send to the user because a none visible
> > > >>> >> component will
> > > >>> >> > > be render as an empty tag, so without data
> > > >>> >> > >
> > > >>> >> > >
> > > >>> >> >
> > > >>> >> >
> > > >>> >> > --
> > > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> > > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
> > now!
> > > >>> >> > http://wicketframework.org
> > > >>> >> >
> > > >>> >>
> > > >>> >
> > > >>> >
> > > >>>
> > > >>>
> > > >>
> > > >>
> > > >
> > > >
> > >
> > >
> >
>
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
As I have explained in my previous mails, and as Igor did in his
messages, you can query the dom for existence of an element:

document.getElementById('foo') == null

The following is legal, and (except for the random bit) a common idiom.

new Label("foo", "Hello, World") { public boolean isVisible() {return
new Random().nextBoolean(); } }.setOutputMarkupId(true);

So, using outputmarkup id does *NOT* necessarily mean the tag should
be rendered.

Martijn


On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> Igor Vaynberg a écrit :
> > no its not.
> >
> > what i am trying to say is that setoutputmarkupid should be
> > independent from
> > this behavior. you are really making the meaning of
> > setoutputmarkupid(true)
> > ambiguous. all it is saying is that when the tag is rendered i want it to
> > include the id attribute, you are changing that meaning to also
> > include the
> > output tag and not leaving me with any option to go back to the original
> > behavior - which is very bad.
> I think if you ask to display the markupid (setoutputmarkupid(true)) and
> setVisible(false) on your component, you will never find in your dom the
> id you asked to display, I think it is also disturbing
> Imho i d'ont think it change the meaning of setoutputmarkupid.
>
> --
> Vincent
> >
> > -igor
> >
> >
> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >>
> >> Igor Vaynberg a écrit :
> >> > what you need is a different method. instead of
> >> > setoutputmarkupid(true) you
> >> > need something that will do
> >> >
> >> > setoutputmarkuptag(boolean b) {
> >> >  if (b) {
> >> >     setoutputmarkupid(true);
> >> >     setflag(outputmarkuptag,true);
> >> >   } else {
> >> >       setflag(outputmarkuptag,false);
> >> >       //not sure if we should undo setoutputmarkupid here
> >> >    }
> >> > }
> >>
> >> Yes, it is the current behavior of the patch ;) actually, the current
> >> behavior is
> >> if outputMarkupid == true
> >>     render componentTag + attribute style and id
> >> else
> >>     render nothing
> >>
> >> so if you want to render nothing just set outputMarkupId to false, for
> >> the behavior we proposed just setoutputMarkupId to true
> >>
> >> It is exactly what you describe in setoutputmarkuptag ;)
> >>
> >> --
> >> Vincent
> >>
> >> >
> >> > now this will solve the problems outlined so far
> >> >
> >> > btw the latest patch attached to the issue is bad because it executes
> >> > behaviors which should not happen.
> >> >
> >> > what i am worried about are javascript libraries and css. does css 3
> >> have
> >> > support for odd/even selectors? that will break. this will even break
> >> > css 2
> >> > first child selector if that happens to be invisible because i dont
> >> > think it
> >> > checks for the display attribute. it will also break  3rd party
> >> > javascript
> >> > libs that dont check for display attrs.
> >> >
> >> > i see how this makes life easier, but it also has a potential to be
> >> evil.
> >> > that is why i closed the issue as wont-fix and told vincent to
> >> bring the
> >> > discussion here on the list before we go any further.
> >> >
> >> > -igor
> >> >
> >> >
> >> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> >>
> >> >> I don't like it. I don't think deprecating set/isVisible is the
> >> way to
> >> >> go. Plus there are other reasons: As now we use one flag for visible
> >> >> status. With your approach you'd require an enum, int, byte or
> >> >> something similiar, that takes more space then just one bit in flags.
> >> >>
> >> >> I think we can just make a getter, that by default returns the value
> >> >> from application settings. And you can override that for your
> >> >> component, if you want something different that what's set in
> >> >> application settings.
> >> >>
> >> >> -Matej
> >> >>
> >> >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >> >> > Frédéric Bertin a écrit :
> >> >> > > Martijn Dashorst wrote:
> >> >> > >>> it seems taht this kind of construction is used to make
> >> workaround
> >> >> of
> >> >> > >>> the bug. Is'n it?
> >> >> > >>
> >> >> > >> First, what bug? I don't know that this is a bug? I thought
> >> we are
> >> >> > >> discussing a feature here. Secondly, this is not a
> >> workaround, but
> >> >> > >> creating client side code based on a API contract:
> >> >> setVisible(false)
> >> >> > >> removes the component markup completely, including its tags,
> >> >> from the
> >> >> > >> final markup.
> >> >> > > ok, then there's a need for 2 different mechanisms.
> >> >> > > One to switch a component visibility. This one should be
> >> >> *reversible*
> >> >> > > (in ajax or not), and then it should always output a tag with
> >> an id
> >> >> > > attribute (actually, can be done only when setOutputMarkupId is
> >> >> set to
> >> >> > > true).
> >> >> > >
> >> >> > > Another to render or not a component in the output markup.
> >> This one
> >> >> > > should be documented as *not reversible*. This is the current
> >> >> > > setVisible implementation.
> >> >> > >
> >> >> > > wdyt?
> >> >> >
> >> >> > +1
> >> >> > What about keeping current behavior on setVisible (deprecated) and
> >> >> > add a method setVisibility :
> >> >> >     - none : render nothing
> >> >> >     - visible : render all
> >> >> >     - invisible : render only container tag with style:display-none
> >> >> >
> >> >> > Add in documentation
> >> >> > none: can not become visible in ajax
> >> >> > in visible: can
> >> >> >
> >> >> > I think it will match to all use case, no ?
> >> >> > >
> >> >> > >
> >> >> > >
> >> >> > >>
> >> >> > >> It is based on the assumption that some element is *NOT* present
> >> at
> >> >> > >> all. Your change will invert that behavior, and in such a way
> >> >> that it
> >> >> > >> is only detectable by debugging your javascript. Not something I
> >> >> > >> enjoy, nor 95% of the development community.
> >> >> > >>
> >> >> > >> You must understand that this is a major api break, not
> >> something
> >> >> > >> minor. This is not detectable by a compiler. You *will* break
> >> >> existing
> >> >> > >> scripts, pages and applications in a non-obvious way. Silent
> >> >> failures
> >> >> > >> are something we try to avoid at all cost.
> >> >> > >>
> >> >> > >> Martijn
> >> >> > >>
> >> >> > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> >> wrote:
> >> >> > >>> Martijn Dashorst a écrit :
> >> >> > >>> > Currently everybody assumes (correctly) that the element is
> >> >> > >>> completely
> >> >> > >>> > removed (Ajax and non-Ajax)
> >> >> > >>> Yes of course, but it is the same for all workarounds ;)
> >> >> > >>> When to change servlet to filter, users have to change their
> >> web
> >> >> xml.
> >> >> > >>> Each time you change something users have to adapt their code
> >> >> > >>> > i.e. not present in the final markup.
> >> >> > >>> > This means that scripts that iterate through the dom, or
> >> >> check for
> >> >> > >>> the
> >> >> > >>> > document.getElementById() == null will fail if we implement
> >> >> this.
> >> >> > >>> >
> >> >> > >>> it seems taht this kind of construction is used to make
> >> workaround
> >> >> of
> >> >> > >>> the bug. Is'n it?
> >> >> > >>> > I *strongly* discourage changing this behavior.
> >> >> > >>> >
> >> >> > >>> > Martijn
> >> >> > >>> >
> >> >> > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> >> > >>> >> Will it? This seems to be actually quite a smart workaround.
> >> >> How
> >> >> > >>> >> exactly will this break existing clients? Only thing i'm
> >> >> concerned
> >> >> > >>> >> about is the validity of output markup. but imho when we
> >> >> preserve
> >> >> > >>> the
> >> >> > >>> >> original tag names, e.g. td will render as td, it should be
> >> all
> >> >> > >>> right.
> >> >> > >>> >>
> >> >> > >>> >> -Matej
> >> >> > >>> >>
> >> >> > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
> >> >> wrote:
> >> >> > >>> >> > So you mean:
> >> >> > >>> >> >
> >> >> > >>> >> >     Label l = Label("foo", "hello");
> >> >> > >>> >> > renders:
> >> >> > >>> >> >     <span wicket:id="foo">hello</span>
> >> >> > >>> >> >
> >> >> > >>> >> > ... some ajax stuff, or a normal page render:
> >> >> > >>> >> >
> >> >> > >>> >> >     l.setVisible(false);
> >> >> > >>> >> > renders:
> >> >> > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> >> >> > >>> >> > ?
> >> >> > >>> >> >
> >> >> > >>> >> > This can and will break existing clients in a very nasty
> >> >> manner,
> >> >> > >>> >> > because the markup id is still present in the final
> >> markup.
> >> >> > >>> >> >
> >> >> > >>> >> > Martijn
> >> >> > >>> >> >
> >> >> > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> >> >> wrote:
> >> >> > >>> >> > > Johan Compagner a écrit :
> >> >> > >>> >> > > >> > Also always just rendering the component but use
> >> the
> >> >> style
> >> >> > >>> >> to make in
> >> >> > >>> >> > > >> > invisible
> >> >> > >>> >> > > >> > could be a security problem. So that can't be the
> >> >> default.
> >> >> > >>> >> > > >>
> >> >> > >>> >> > > >> What do you mean by security problem?
> >> >> > >>> >> > > >
> >> >> > >>> >> > > >
> >> >> > >>> >> > > >
> >> >> > >>> >> > > > If the the component that is  set to none visible is
> >> none
> >> >> > >>> visible
> >> >> > >>> >> > > > because of
> >> >> > >>> >> > > > security
> >> >> > >>> >> > > > So it has data that never should be send to the
> >> browser
> >> >> > >>> because
> >> >> > >>> >> the
> >> >> > >>> >> > > > user is
> >> >> > >>> >> > > > not allowed
> >> >> > >>> >> > > > to see it.
> >> >> > >>> >> > >
> >> >> > >>> >> > > But data is never send to the user because a none
> >> visible
> >> >> > >>> >> component will
> >> >> > >>> >> > > be render as an empty tag, so without data
> >> >> > >>> >> > >
> >> >> > >>> >> > >
> >> >> > >>> >> >
> >> >> > >>> >> >
> >> >> > >>> >> > --
> >> >> > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> >> >> > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> >> >> > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
> >> >> now!
> >> >> > >>> >> > http://wicketframework.org
> >> >> > >>> >> >
> >> >> > >>> >>
> >> >> > >>> >
> >> >> > >>> >
> >> >> > >>>
> >> >> > >>>
> >> >> > >>
> >> >> > >>
> >> >> > >
> >> >> > >
> >> >> >
> >> >> >
> >> >>
> >> >
> >>
> >>
> >
>
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Igor Vaynberg a écrit :
> you have to get out of your ajax tunnel vision.
>
> take the datepicker example. it needs the outputmarkupid(true) so that it
> can tie the datepicker js to the textfield. but when the datepicker is 
> not
> visible why should an empty tag still be there?

ok, so +1 for your code snippet.
If i want to discuss again about that it was to find the better way to 
go from invisible to visible in ajax. Sorry for my "ajax tunnel vision" 
;) but I think every vision are important in a chat,  I think we have 
now something that everybody feels acceptable.

Thanks

--
Vincent

>
> -igor
>
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>>
>> Igor Vaynberg a écrit :
>> > no its not.
>> >
>> > what i am trying to say is that setoutputmarkupid should be
>> > independent from
>> > this behavior. you are really making the meaning of
>> > setoutputmarkupid(true)
>> > ambiguous. all it is saying is that when the tag is rendered i want it
>> to
>> > include the id attribute, you are changing that meaning to also
>> > include the
>> > output tag and not leaving me with any option to go back to the 
>> original
>> > behavior - which is very bad.
>> I think if you ask to display the markupid (setoutputmarkupid(true)) and
>> setVisible(false) on your component, you will never find in your dom the
>> id you asked to display, I think it is also disturbing
>> Imho i d'ont think it change the meaning of setoutputmarkupid.
>>
>> -- 
>> Vincent
>> >
>> > -igor
>> >
>> >
>> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> >>
>> >> Igor Vaynberg a écrit :
>> >> > what you need is a different method. instead of
>> >> > setoutputmarkupid(true) you
>> >> > need something that will do
>> >> >
>> >> > setoutputmarkuptag(boolean b) {
>> >> >  if (b) {
>> >> >     setoutputmarkupid(true);
>> >> >     setflag(outputmarkuptag,true);
>> >> >   } else {
>> >> >       setflag(outputmarkuptag,false);
>> >> >       //not sure if we should undo setoutputmarkupid here
>> >> >    }
>> >> > }
>> >>
>> >> Yes, it is the current behavior of the patch ;) actually, the current
>> >> behavior is
>> >> if outputMarkupid == true
>> >>     render componentTag + attribute style and id
>> >> else
>> >>     render nothing
>> >>
>> >> so if you want to render nothing just set outputMarkupId to false, 
>> for
>> >> the behavior we proposed just setoutputMarkupId to true
>> >>
>> >> It is exactly what you describe in setoutputmarkuptag ;)
>> >>
>> >> --
>> >> Vincent
>> >>
>> >> >
>> >> > now this will solve the problems outlined so far
>> >> >
>> >> > btw the latest patch attached to the issue is bad because it 
>> executes
>> >> > behaviors which should not happen.
>> >> >
>> >> > what i am worried about are javascript libraries and css. does 
>> css 3
>> >> have
>> >> > support for odd/even selectors? that will break. this will even 
>> break
>> >> > css 2
>> >> > first child selector if that happens to be invisible because i dont
>> >> > think it
>> >> > checks for the display attribute. it will also break  3rd party
>> >> > javascript
>> >> > libs that dont check for display attrs.
>> >> >
>> >> > i see how this makes life easier, but it also has a potential to be
>> >> evil.
>> >> > that is why i closed the issue as wont-fix and told vincent to
>> >> bring the
>> >> > discussion here on the list before we go any further.
>> >> >
>> >> > -igor
>> >> >
>> >> >
>> >> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >> >>
>> >> >> I don't like it. I don't think deprecating set/isVisible is the
>> >> way to
>> >> >> go. Plus there are other reasons: As now we use one flag for 
>> visible
>> >> >> status. With your approach you'd require an enum, int, byte or
>> >> >> something similiar, that takes more space then just one bit in
>> flags.
>> >> >>
>> >> >> I think we can just make a getter, that by default returns the 
>> value
>> >> >> from application settings. And you can override that for your
>> >> >> component, if you want something different that what's set in
>> >> >> application settings.
>> >> >>
>> >> >> -Matej
>> >> >>
>> >> >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> >> >> > Frédéric Bertin a écrit :
>> >> >> > > Martijn Dashorst wrote:
>> >> >> > >>> it seems taht this kind of construction is used to make
>> >> workaround
>> >> >> of
>> >> >> > >>> the bug. Is'n it?
>> >> >> > >>
>> >> >> > >> First, what bug? I don't know that this is a bug? I thought
>> >> we are
>> >> >> > >> discussing a feature here. Secondly, this is not a
>> >> workaround, but
>> >> >> > >> creating client side code based on a API contract:
>> >> >> setVisible(false)
>> >> >> > >> removes the component markup completely, including its tags,
>> >> >> from the
>> >> >> > >> final markup.
>> >> >> > > ok, then there's a need for 2 different mechanisms.
>> >> >> > > One to switch a component visibility. This one should be
>> >> >> *reversible*
>> >> >> > > (in ajax or not), and then it should always output a tag with
>> >> an id
>> >> >> > > attribute (actually, can be done only when 
>> setOutputMarkupId is
>> >> >> set to
>> >> >> > > true).
>> >> >> > >
>> >> >> > > Another to render or not a component in the output markup.
>> >> This one
>> >> >> > > should be documented as *not reversible*. This is the current
>> >> >> > > setVisible implementation.
>> >> >> > >
>> >> >> > > wdyt?
>> >> >> >
>> >> >> > +1
>> >> >> > What about keeping current behavior on setVisible 
>> (deprecated) and
>> >> >> > add a method setVisibility :
>> >> >> >     - none : render nothing
>> >> >> >     - visible : render all
>> >> >> >     - invisible : render only container tag with
>> style:display-none
>> >> >> >
>> >> >> > Add in documentation
>> >> >> > none: can not become visible in ajax
>> >> >> > in visible: can
>> >> >> >
>> >> >> > I think it will match to all use case, no ?
>> >> >> > >
>> >> >> > >
>> >> >> > >
>> >> >> > >>
>> >> >> > >> It is based on the assumption that some element is *NOT*
>> present
>> >> at
>> >> >> > >> all. Your change will invert that behavior, and in such a way
>> >> >> that it
>> >> >> > >> is only detectable by debugging your javascript. Not 
>> something
>> I
>> >> >> > >> enjoy, nor 95% of the development community.
>> >> >> > >>
>> >> >> > >> You must understand that this is a major api break, not
>> >> something
>> >> >> > >> minor. This is not detectable by a compiler. You *will* break
>> >> >> existing
>> >> >> > >> scripts, pages and applications in a non-obvious way. Silent
>> >> >> failures
>> >> >> > >> are something we try to avoid at all cost.
>> >> >> > >>
>> >> >> > >> Martijn
>> >> >> > >>
>> >> >> > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
>> >> wrote:
>> >> >> > >>> Martijn Dashorst a écrit :
>> >> >> > >>> > Currently everybody assumes (correctly) that the 
>> element is
>> >> >> > >>> completely
>> >> >> > >>> > removed (Ajax and non-Ajax)
>> >> >> > >>> Yes of course, but it is the same for all workarounds ;)
>> >> >> > >>> When to change servlet to filter, users have to change their
>> >> web
>> >> >> xml.
>> >> >> > >>> Each time you change something users have to adapt their 
>> code
>> >> >> > >>> > i.e. not present in the final markup.
>> >> >> > >>> > This means that scripts that iterate through the dom, or
>> >> >> check for
>> >> >> > >>> the
>> >> >> > >>> > document.getElementById() == null will fail if we 
>> implement
>> >> >> this.
>> >> >> > >>> >
>> >> >> > >>> it seems taht this kind of construction is used to make
>> >> workaround
>> >> >> of
>> >> >> > >>> the bug. Is'n it?
>> >> >> > >>> > I *strongly* discourage changing this behavior.
>> >> >> > >>> >
>> >> >> > >>> > Martijn
>> >> >> > >>> >
>> >> >> > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >> >> > >>> >> Will it? This seems to be actually quite a smart
>> workaround.
>> >> >> How
>> >> >> > >>> >> exactly will this break existing clients? Only thing i'm
>> >> >> concerned
>> >> >> > >>> >> about is the validity of output markup. but imho when we
>> >> >> preserve
>> >> >> > >>> the
>> >> >> > >>> >> original tag names, e.g. td will render as td, it 
>> should be
>> >> all
>> >> >> > >>> right.
>> >> >> > >>> >>
>> >> >> > >>> >> -Matej
>> >> >> > >>> >>
>> >> >> > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
>> >> >> wrote:
>> >> >> > >>> >> > So you mean:
>> >> >> > >>> >> >
>> >> >> > >>> >> >     Label l = Label("foo", "hello");
>> >> >> > >>> >> > renders:
>> >> >> > >>> >> >     <span wicket:id="foo">hello</span>
>> >> >> > >>> >> >
>> >> >> > >>> >> > ... some ajax stuff, or a normal page render:
>> >> >> > >>> >> >
>> >> >> > >>> >> >     l.setVisible(false);
>> >> >> > >>> >> > renders:
>> >> >> > >>> >> >     <span wicket:id="foo" style="display:none"></span>
>> >> >> > >>> >> > ?
>> >> >> > >>> >> >
>> >> >> > >>> >> > This can and will break existing clients in a very 
>> nasty
>> >> >> manner,
>> >> >> > >>> >> > because the markup id is still present in the final
>> >> markup.
>> >> >> > >>> >> >
>> >> >> > >>> >> > Martijn
>> >> >> > >>> >> >
>> >> >> > >>> >> > On 3/19/07, Vincent Demay 
>> <vincent.demay@anyware-tech.com
>> >
>> >> >> wrote:
>> >> >> > >>> >> > > Johan Compagner a écrit :
>> >> >> > >>> >> > > >> > Also always just rendering the component but use
>> >> the
>> >> >> style
>> >> >> > >>> >> to make in
>> >> >> > >>> >> > > >> > invisible
>> >> >> > >>> >> > > >> > could be a security problem. So that can't be 
>> the
>> >> >> default.
>> >> >> > >>> >> > > >>
>> >> >> > >>> >> > > >> What do you mean by security problem?
>> >> >> > >>> >> > > >
>> >> >> > >>> >> > > >
>> >> >> > >>> >> > > >
>> >> >> > >>> >> > > > If the the component that is  set to none 
>> visible is
>> >> none
>> >> >> > >>> visible
>> >> >> > >>> >> > > > because of
>> >> >> > >>> >> > > > security
>> >> >> > >>> >> > > > So it has data that never should be send to the
>> >> browser
>> >> >> > >>> because
>> >> >> > >>> >> the
>> >> >> > >>> >> > > > user is
>> >> >> > >>> >> > > > not allowed
>> >> >> > >>> >> > > > to see it.
>> >> >> > >>> >> > >
>> >> >> > >>> >> > > But data is never send to the user because a none
>> >> visible
>> >> >> > >>> >> component will
>> >> >> > >>> >> > > be render as an empty tag, so without data
>> >> >> > >>> >> > >
>> >> >> > >>> >> > >
>> >> >> > >>> >> >
>> >> >> > >>> >> >
>> >> >> > >>> >> > --
>> >> >> > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> >> >> > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> >> >> > >>> >> > Wicket 1.2.5 will keep your server alive. Download 
>> Wicket
>> >> >> now!
>> >> >> > >>> >> > http://wicketframework.org
>> >> >> > >>> >> >
>> >> >> > >>> >>
>> >> >> > >>> >
>> >> >> > >>> >
>> >> >> > >>>
>> >> >> > >>>
>> >> >> > >>
>> >> >> > >>
>> >> >> > >
>> >> >> > >
>> >> >> >
>> >> >> >
>> >> >>
>> >> >
>> >>
>> >>
>> >
>>
>>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Igor Vaynberg <ig...@gmail.com>.
you have to get out of your ajax tunnel vision.

take the datepicker example. it needs the outputmarkupid(true) so that it
can tie the datepicker js to the textfield. but when the datepicker is not
visible why should an empty tag still be there?

-igor


On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>
> Igor Vaynberg a écrit :
> > no its not.
> >
> > what i am trying to say is that setoutputmarkupid should be
> > independent from
> > this behavior. you are really making the meaning of
> > setoutputmarkupid(true)
> > ambiguous. all it is saying is that when the tag is rendered i want it
> to
> > include the id attribute, you are changing that meaning to also
> > include the
> > output tag and not leaving me with any option to go back to the original
> > behavior - which is very bad.
> I think if you ask to display the markupid (setoutputmarkupid(true)) and
> setVisible(false) on your component, you will never find in your dom the
> id you asked to display, I think it is also disturbing
> Imho i d'ont think it change the meaning of setoutputmarkupid.
>
> --
> Vincent
> >
> > -igor
> >
> >
> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >>
> >> Igor Vaynberg a écrit :
> >> > what you need is a different method. instead of
> >> > setoutputmarkupid(true) you
> >> > need something that will do
> >> >
> >> > setoutputmarkuptag(boolean b) {
> >> >  if (b) {
> >> >     setoutputmarkupid(true);
> >> >     setflag(outputmarkuptag,true);
> >> >   } else {
> >> >       setflag(outputmarkuptag,false);
> >> >       //not sure if we should undo setoutputmarkupid here
> >> >    }
> >> > }
> >>
> >> Yes, it is the current behavior of the patch ;) actually, the current
> >> behavior is
> >> if outputMarkupid == true
> >>     render componentTag + attribute style and id
> >> else
> >>     render nothing
> >>
> >> so if you want to render nothing just set outputMarkupId to false, for
> >> the behavior we proposed just setoutputMarkupId to true
> >>
> >> It is exactly what you describe in setoutputmarkuptag ;)
> >>
> >> --
> >> Vincent
> >>
> >> >
> >> > now this will solve the problems outlined so far
> >> >
> >> > btw the latest patch attached to the issue is bad because it executes
> >> > behaviors which should not happen.
> >> >
> >> > what i am worried about are javascript libraries and css. does css 3
> >> have
> >> > support for odd/even selectors? that will break. this will even break
> >> > css 2
> >> > first child selector if that happens to be invisible because i dont
> >> > think it
> >> > checks for the display attribute. it will also break  3rd party
> >> > javascript
> >> > libs that dont check for display attrs.
> >> >
> >> > i see how this makes life easier, but it also has a potential to be
> >> evil.
> >> > that is why i closed the issue as wont-fix and told vincent to
> >> bring the
> >> > discussion here on the list before we go any further.
> >> >
> >> > -igor
> >> >
> >> >
> >> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> >>
> >> >> I don't like it. I don't think deprecating set/isVisible is the
> >> way to
> >> >> go. Plus there are other reasons: As now we use one flag for visible
> >> >> status. With your approach you'd require an enum, int, byte or
> >> >> something similiar, that takes more space then just one bit in
> flags.
> >> >>
> >> >> I think we can just make a getter, that by default returns the value
> >> >> from application settings. And you can override that for your
> >> >> component, if you want something different that what's set in
> >> >> application settings.
> >> >>
> >> >> -Matej
> >> >>
> >> >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >> >> > Frédéric Bertin a écrit :
> >> >> > > Martijn Dashorst wrote:
> >> >> > >>> it seems taht this kind of construction is used to make
> >> workaround
> >> >> of
> >> >> > >>> the bug. Is'n it?
> >> >> > >>
> >> >> > >> First, what bug? I don't know that this is a bug? I thought
> >> we are
> >> >> > >> discussing a feature here. Secondly, this is not a
> >> workaround, but
> >> >> > >> creating client side code based on a API contract:
> >> >> setVisible(false)
> >> >> > >> removes the component markup completely, including its tags,
> >> >> from the
> >> >> > >> final markup.
> >> >> > > ok, then there's a need for 2 different mechanisms.
> >> >> > > One to switch a component visibility. This one should be
> >> >> *reversible*
> >> >> > > (in ajax or not), and then it should always output a tag with
> >> an id
> >> >> > > attribute (actually, can be done only when setOutputMarkupId is
> >> >> set to
> >> >> > > true).
> >> >> > >
> >> >> > > Another to render or not a component in the output markup.
> >> This one
> >> >> > > should be documented as *not reversible*. This is the current
> >> >> > > setVisible implementation.
> >> >> > >
> >> >> > > wdyt?
> >> >> >
> >> >> > +1
> >> >> > What about keeping current behavior on setVisible (deprecated) and
> >> >> > add a method setVisibility :
> >> >> >     - none : render nothing
> >> >> >     - visible : render all
> >> >> >     - invisible : render only container tag with
> style:display-none
> >> >> >
> >> >> > Add in documentation
> >> >> > none: can not become visible in ajax
> >> >> > in visible: can
> >> >> >
> >> >> > I think it will match to all use case, no ?
> >> >> > >
> >> >> > >
> >> >> > >
> >> >> > >>
> >> >> > >> It is based on the assumption that some element is *NOT*
> present
> >> at
> >> >> > >> all. Your change will invert that behavior, and in such a way
> >> >> that it
> >> >> > >> is only detectable by debugging your javascript. Not something
> I
> >> >> > >> enjoy, nor 95% of the development community.
> >> >> > >>
> >> >> > >> You must understand that this is a major api break, not
> >> something
> >> >> > >> minor. This is not detectable by a compiler. You *will* break
> >> >> existing
> >> >> > >> scripts, pages and applications in a non-obvious way. Silent
> >> >> failures
> >> >> > >> are something we try to avoid at all cost.
> >> >> > >>
> >> >> > >> Martijn
> >> >> > >>
> >> >> > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> >> wrote:
> >> >> > >>> Martijn Dashorst a écrit :
> >> >> > >>> > Currently everybody assumes (correctly) that the element is
> >> >> > >>> completely
> >> >> > >>> > removed (Ajax and non-Ajax)
> >> >> > >>> Yes of course, but it is the same for all workarounds ;)
> >> >> > >>> When to change servlet to filter, users have to change their
> >> web
> >> >> xml.
> >> >> > >>> Each time you change something users have to adapt their code
> >> >> > >>> > i.e. not present in the final markup.
> >> >> > >>> > This means that scripts that iterate through the dom, or
> >> >> check for
> >> >> > >>> the
> >> >> > >>> > document.getElementById() == null will fail if we implement
> >> >> this.
> >> >> > >>> >
> >> >> > >>> it seems taht this kind of construction is used to make
> >> workaround
> >> >> of
> >> >> > >>> the bug. Is'n it?
> >> >> > >>> > I *strongly* discourage changing this behavior.
> >> >> > >>> >
> >> >> > >>> > Martijn
> >> >> > >>> >
> >> >> > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> >> > >>> >> Will it? This seems to be actually quite a smart
> workaround.
> >> >> How
> >> >> > >>> >> exactly will this break existing clients? Only thing i'm
> >> >> concerned
> >> >> > >>> >> about is the validity of output markup. but imho when we
> >> >> preserve
> >> >> > >>> the
> >> >> > >>> >> original tag names, e.g. td will render as td, it should be
> >> all
> >> >> > >>> right.
> >> >> > >>> >>
> >> >> > >>> >> -Matej
> >> >> > >>> >>
> >> >> > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
> >> >> wrote:
> >> >> > >>> >> > So you mean:
> >> >> > >>> >> >
> >> >> > >>> >> >     Label l = Label("foo", "hello");
> >> >> > >>> >> > renders:
> >> >> > >>> >> >     <span wicket:id="foo">hello</span>
> >> >> > >>> >> >
> >> >> > >>> >> > ... some ajax stuff, or a normal page render:
> >> >> > >>> >> >
> >> >> > >>> >> >     l.setVisible(false);
> >> >> > >>> >> > renders:
> >> >> > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> >> >> > >>> >> > ?
> >> >> > >>> >> >
> >> >> > >>> >> > This can and will break existing clients in a very nasty
> >> >> manner,
> >> >> > >>> >> > because the markup id is still present in the final
> >> markup.
> >> >> > >>> >> >
> >> >> > >>> >> > Martijn
> >> >> > >>> >> >
> >> >> > >>> >> > On 3/19/07, Vincent Demay <vincent.demay@anyware-tech.com
> >
> >> >> wrote:
> >> >> > >>> >> > > Johan Compagner a écrit :
> >> >> > >>> >> > > >> > Also always just rendering the component but use
> >> the
> >> >> style
> >> >> > >>> >> to make in
> >> >> > >>> >> > > >> > invisible
> >> >> > >>> >> > > >> > could be a security problem. So that can't be the
> >> >> default.
> >> >> > >>> >> > > >>
> >> >> > >>> >> > > >> What do you mean by security problem?
> >> >> > >>> >> > > >
> >> >> > >>> >> > > >
> >> >> > >>> >> > > >
> >> >> > >>> >> > > > If the the component that is  set to none visible is
> >> none
> >> >> > >>> visible
> >> >> > >>> >> > > > because of
> >> >> > >>> >> > > > security
> >> >> > >>> >> > > > So it has data that never should be send to the
> >> browser
> >> >> > >>> because
> >> >> > >>> >> the
> >> >> > >>> >> > > > user is
> >> >> > >>> >> > > > not allowed
> >> >> > >>> >> > > > to see it.
> >> >> > >>> >> > >
> >> >> > >>> >> > > But data is never send to the user because a none
> >> visible
> >> >> > >>> >> component will
> >> >> > >>> >> > > be render as an empty tag, so without data
> >> >> > >>> >> > >
> >> >> > >>> >> > >
> >> >> > >>> >> >
> >> >> > >>> >> >
> >> >> > >>> >> > --
> >> >> > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> >> >> > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> >> >> > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
> >> >> now!
> >> >> > >>> >> > http://wicketframework.org
> >> >> > >>> >> >
> >> >> > >>> >>
> >> >> > >>> >
> >> >> > >>> >
> >> >> > >>>
> >> >> > >>>
> >> >> > >>
> >> >> > >>
> >> >> > >
> >> >> > >
> >> >> >
> >> >> >
> >> >>
> >> >
> >>
> >>
> >
>
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Igor Vaynberg a écrit :
> no its not.
>
> what i am trying to say is that setoutputmarkupid should be 
> independent from
> this behavior. you are really making the meaning of 
> setoutputmarkupid(true)
> ambiguous. all it is saying is that when the tag is rendered i want it to
> include the id attribute, you are changing that meaning to also 
> include the
> output tag and not leaving me with any option to go back to the original
> behavior - which is very bad.
I think if you ask to display the markupid (setoutputmarkupid(true)) and 
setVisible(false) on your component, you will never find in your dom the 
id you asked to display, I think it is also disturbing
Imho i d'ont think it change the meaning of setoutputmarkupid.

--
Vincent
>
> -igor
>
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>>
>> Igor Vaynberg a écrit :
>> > what you need is a different method. instead of
>> > setoutputmarkupid(true) you
>> > need something that will do
>> >
>> > setoutputmarkuptag(boolean b) {
>> >  if (b) {
>> >     setoutputmarkupid(true);
>> >     setflag(outputmarkuptag,true);
>> >   } else {
>> >       setflag(outputmarkuptag,false);
>> >       //not sure if we should undo setoutputmarkupid here
>> >    }
>> > }
>>
>> Yes, it is the current behavior of the patch ;) actually, the current
>> behavior is
>> if outputMarkupid == true
>>     render componentTag + attribute style and id
>> else
>>     render nothing
>>
>> so if you want to render nothing just set outputMarkupId to false, for
>> the behavior we proposed just setoutputMarkupId to true
>>
>> It is exactly what you describe in setoutputmarkuptag ;)
>>
>> -- 
>> Vincent
>>
>> >
>> > now this will solve the problems outlined so far
>> >
>> > btw the latest patch attached to the issue is bad because it executes
>> > behaviors which should not happen.
>> >
>> > what i am worried about are javascript libraries and css. does css 3
>> have
>> > support for odd/even selectors? that will break. this will even break
>> > css 2
>> > first child selector if that happens to be invisible because i dont
>> > think it
>> > checks for the display attribute. it will also break  3rd party
>> > javascript
>> > libs that dont check for display attrs.
>> >
>> > i see how this makes life easier, but it also has a potential to be
>> evil.
>> > that is why i closed the issue as wont-fix and told vincent to 
>> bring the
>> > discussion here on the list before we go any further.
>> >
>> > -igor
>> >
>> >
>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >>
>> >> I don't like it. I don't think deprecating set/isVisible is the 
>> way to
>> >> go. Plus there are other reasons: As now we use one flag for visible
>> >> status. With your approach you'd require an enum, int, byte or
>> >> something similiar, that takes more space then just one bit in flags.
>> >>
>> >> I think we can just make a getter, that by default returns the value
>> >> from application settings. And you can override that for your
>> >> component, if you want something different that what's set in
>> >> application settings.
>> >>
>> >> -Matej
>> >>
>> >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> >> > Frédéric Bertin a écrit :
>> >> > > Martijn Dashorst wrote:
>> >> > >>> it seems taht this kind of construction is used to make
>> workaround
>> >> of
>> >> > >>> the bug. Is'n it?
>> >> > >>
>> >> > >> First, what bug? I don't know that this is a bug? I thought 
>> we are
>> >> > >> discussing a feature here. Secondly, this is not a 
>> workaround, but
>> >> > >> creating client side code based on a API contract:
>> >> setVisible(false)
>> >> > >> removes the component markup completely, including its tags,
>> >> from the
>> >> > >> final markup.
>> >> > > ok, then there's a need for 2 different mechanisms.
>> >> > > One to switch a component visibility. This one should be
>> >> *reversible*
>> >> > > (in ajax or not), and then it should always output a tag with 
>> an id
>> >> > > attribute (actually, can be done only when setOutputMarkupId is
>> >> set to
>> >> > > true).
>> >> > >
>> >> > > Another to render or not a component in the output markup. 
>> This one
>> >> > > should be documented as *not reversible*. This is the current
>> >> > > setVisible implementation.
>> >> > >
>> >> > > wdyt?
>> >> >
>> >> > +1
>> >> > What about keeping current behavior on setVisible (deprecated) and
>> >> > add a method setVisibility :
>> >> >     - none : render nothing
>> >> >     - visible : render all
>> >> >     - invisible : render only container tag with style:display-none
>> >> >
>> >> > Add in documentation
>> >> > none: can not become visible in ajax
>> >> > in visible: can
>> >> >
>> >> > I think it will match to all use case, no ?
>> >> > >
>> >> > >
>> >> > >
>> >> > >>
>> >> > >> It is based on the assumption that some element is *NOT* present
>> at
>> >> > >> all. Your change will invert that behavior, and in such a way
>> >> that it
>> >> > >> is only detectable by debugging your javascript. Not something I
>> >> > >> enjoy, nor 95% of the development community.
>> >> > >>
>> >> > >> You must understand that this is a major api break, not 
>> something
>> >> > >> minor. This is not detectable by a compiler. You *will* break
>> >> existing
>> >> > >> scripts, pages and applications in a non-obvious way. Silent
>> >> failures
>> >> > >> are something we try to avoid at all cost.
>> >> > >>
>> >> > >> Martijn
>> >> > >>
>> >> > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> 
>> wrote:
>> >> > >>> Martijn Dashorst a écrit :
>> >> > >>> > Currently everybody assumes (correctly) that the element is
>> >> > >>> completely
>> >> > >>> > removed (Ajax and non-Ajax)
>> >> > >>> Yes of course, but it is the same for all workarounds ;)
>> >> > >>> When to change servlet to filter, users have to change their 
>> web
>> >> xml.
>> >> > >>> Each time you change something users have to adapt their code
>> >> > >>> > i.e. not present in the final markup.
>> >> > >>> > This means that scripts that iterate through the dom, or
>> >> check for
>> >> > >>> the
>> >> > >>> > document.getElementById() == null will fail if we implement
>> >> this.
>> >> > >>> >
>> >> > >>> it seems taht this kind of construction is used to make
>> workaround
>> >> of
>> >> > >>> the bug. Is'n it?
>> >> > >>> > I *strongly* discourage changing this behavior.
>> >> > >>> >
>> >> > >>> > Martijn
>> >> > >>> >
>> >> > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >> > >>> >> Will it? This seems to be actually quite a smart workaround.
>> >> How
>> >> > >>> >> exactly will this break existing clients? Only thing i'm
>> >> concerned
>> >> > >>> >> about is the validity of output markup. but imho when we
>> >> preserve
>> >> > >>> the
>> >> > >>> >> original tag names, e.g. td will render as td, it should be
>> all
>> >> > >>> right.
>> >> > >>> >>
>> >> > >>> >> -Matej
>> >> > >>> >>
>> >> > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
>> >> wrote:
>> >> > >>> >> > So you mean:
>> >> > >>> >> >
>> >> > >>> >> >     Label l = Label("foo", "hello");
>> >> > >>> >> > renders:
>> >> > >>> >> >     <span wicket:id="foo">hello</span>
>> >> > >>> >> >
>> >> > >>> >> > ... some ajax stuff, or a normal page render:
>> >> > >>> >> >
>> >> > >>> >> >     l.setVisible(false);
>> >> > >>> >> > renders:
>> >> > >>> >> >     <span wicket:id="foo" style="display:none"></span>
>> >> > >>> >> > ?
>> >> > >>> >> >
>> >> > >>> >> > This can and will break existing clients in a very nasty
>> >> manner,
>> >> > >>> >> > because the markup id is still present in the final 
>> markup.
>> >> > >>> >> >
>> >> > >>> >> > Martijn
>> >> > >>> >> >
>> >> > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
>> >> wrote:
>> >> > >>> >> > > Johan Compagner a écrit :
>> >> > >>> >> > > >> > Also always just rendering the component but use 
>> the
>> >> style
>> >> > >>> >> to make in
>> >> > >>> >> > > >> > invisible
>> >> > >>> >> > > >> > could be a security problem. So that can't be the
>> >> default.
>> >> > >>> >> > > >>
>> >> > >>> >> > > >> What do you mean by security problem?
>> >> > >>> >> > > >
>> >> > >>> >> > > >
>> >> > >>> >> > > >
>> >> > >>> >> > > > If the the component that is  set to none visible is
>> none
>> >> > >>> visible
>> >> > >>> >> > > > because of
>> >> > >>> >> > > > security
>> >> > >>> >> > > > So it has data that never should be send to the 
>> browser
>> >> > >>> because
>> >> > >>> >> the
>> >> > >>> >> > > > user is
>> >> > >>> >> > > > not allowed
>> >> > >>> >> > > > to see it.
>> >> > >>> >> > >
>> >> > >>> >> > > But data is never send to the user because a none 
>> visible
>> >> > >>> >> component will
>> >> > >>> >> > > be render as an empty tag, so without data
>> >> > >>> >> > >
>> >> > >>> >> > >
>> >> > >>> >> >
>> >> > >>> >> >
>> >> > >>> >> > --
>> >> > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> >> > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> >> > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
>> >> now!
>> >> > >>> >> > http://wicketframework.org
>> >> > >>> >> >
>> >> > >>> >>
>> >> > >>> >
>> >> > >>> >
>> >> > >>>
>> >> > >>>
>> >> > >>
>> >> > >>
>> >> > >
>> >> > >
>> >> >
>> >> >
>> >>
>> >
>>
>>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Igor Vaynberg <ig...@gmail.com>.
no its not.

what i am trying to say is that setoutputmarkupid should be independent from
this behavior. you are really making the meaning of setoutputmarkupid(true)
ambiguous. all it is saying is that when the tag is rendered i want it to
include the id attribute, you are changing that meaning to also include the
output tag and not leaving me with any option to go back to the original
behavior - which is very bad.

-igor


On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>
> Igor Vaynberg a écrit :
> > what you need is a different method. instead of
> > setoutputmarkupid(true) you
> > need something that will do
> >
> > setoutputmarkuptag(boolean b) {
> >  if (b) {
> >     setoutputmarkupid(true);
> >     setflag(outputmarkuptag,true);
> >   } else {
> >       setflag(outputmarkuptag,false);
> >       //not sure if we should undo setoutputmarkupid here
> >    }
> > }
>
> Yes, it is the current behavior of the patch ;) actually, the current
> behavior is
> if outputMarkupid == true
>     render componentTag + attribute style and id
> else
>     render nothing
>
> so if you want to render nothing just set outputMarkupId to false, for
> the behavior we proposed just setoutputMarkupId to true
>
> It is exactly what you describe in setoutputmarkuptag ;)
>
> --
> Vincent
>
> >
> > now this will solve the problems outlined so far
> >
> > btw the latest patch attached to the issue is bad because it executes
> > behaviors which should not happen.
> >
> > what i am worried about are javascript libraries and css. does css 3
> have
> > support for odd/even selectors? that will break. this will even break
> > css 2
> > first child selector if that happens to be invisible because i dont
> > think it
> > checks for the display attribute. it will also break  3rd party
> > javascript
> > libs that dont check for display attrs.
> >
> > i see how this makes life easier, but it also has a potential to be
> evil.
> > that is why i closed the issue as wont-fix and told vincent to bring the
> > discussion here on the list before we go any further.
> >
> > -igor
> >
> >
> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >>
> >> I don't like it. I don't think deprecating set/isVisible is the way to
> >> go. Plus there are other reasons: As now we use one flag for visible
> >> status. With your approach you'd require an enum, int, byte or
> >> something similiar, that takes more space then just one bit in flags.
> >>
> >> I think we can just make a getter, that by default returns the value
> >> from application settings. And you can override that for your
> >> component, if you want something different that what's set in
> >> application settings.
> >>
> >> -Matej
> >>
> >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >> > Frédéric Bertin a écrit :
> >> > > Martijn Dashorst wrote:
> >> > >>> it seems taht this kind of construction is used to make
> workaround
> >> of
> >> > >>> the bug. Is'n it?
> >> > >>
> >> > >> First, what bug? I don't know that this is a bug? I thought we are
> >> > >> discussing a feature here. Secondly, this is not a workaround, but
> >> > >> creating client side code based on a API contract:
> >> setVisible(false)
> >> > >> removes the component markup completely, including its tags,
> >> from the
> >> > >> final markup.
> >> > > ok, then there's a need for 2 different mechanisms.
> >> > > One to switch a component visibility. This one should be
> >> *reversible*
> >> > > (in ajax or not), and then it should always output a tag with an id
> >> > > attribute (actually, can be done only when setOutputMarkupId is
> >> set to
> >> > > true).
> >> > >
> >> > > Another to render or not a component in the output markup. This one
> >> > > should be documented as *not reversible*. This is the current
> >> > > setVisible implementation.
> >> > >
> >> > > wdyt?
> >> >
> >> > +1
> >> > What about keeping current behavior on setVisible (deprecated) and
> >> > add a method setVisibility :
> >> >     - none : render nothing
> >> >     - visible : render all
> >> >     - invisible : render only container tag with style:display-none
> >> >
> >> > Add in documentation
> >> > none: can not become visible in ajax
> >> > in visible: can
> >> >
> >> > I think it will match to all use case, no ?
> >> > >
> >> > >
> >> > >
> >> > >>
> >> > >> It is based on the assumption that some element is *NOT* present
> at
> >> > >> all. Your change will invert that behavior, and in such a way
> >> that it
> >> > >> is only detectable by debugging your javascript. Not something I
> >> > >> enjoy, nor 95% of the development community.
> >> > >>
> >> > >> You must understand that this is a major api break, not something
> >> > >> minor. This is not detectable by a compiler. You *will* break
> >> existing
> >> > >> scripts, pages and applications in a non-obvious way. Silent
> >> failures
> >> > >> are something we try to avoid at all cost.
> >> > >>
> >> > >> Martijn
> >> > >>
> >> > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >> > >>> Martijn Dashorst a écrit :
> >> > >>> > Currently everybody assumes (correctly) that the element is
> >> > >>> completely
> >> > >>> > removed (Ajax and non-Ajax)
> >> > >>> Yes of course, but it is the same for all workarounds ;)
> >> > >>> When to change servlet to filter, users have to change their web
> >> xml.
> >> > >>> Each time you change something users have to adapt their code
> >> > >>> > i.e. not present in the final markup.
> >> > >>> > This means that scripts that iterate through the dom, or
> >> check for
> >> > >>> the
> >> > >>> > document.getElementById() == null will fail if we implement
> >> this.
> >> > >>> >
> >> > >>> it seems taht this kind of construction is used to make
> workaround
> >> of
> >> > >>> the bug. Is'n it?
> >> > >>> > I *strongly* discourage changing this behavior.
> >> > >>> >
> >> > >>> > Martijn
> >> > >>> >
> >> > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> > >>> >> Will it? This seems to be actually quite a smart workaround.
> >> How
> >> > >>> >> exactly will this break existing clients? Only thing i'm
> >> concerned
> >> > >>> >> about is the validity of output markup. but imho when we
> >> preserve
> >> > >>> the
> >> > >>> >> original tag names, e.g. td will render as td, it should be
> all
> >> > >>> right.
> >> > >>> >>
> >> > >>> >> -Matej
> >> > >>> >>
> >> > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com>
> >> wrote:
> >> > >>> >> > So you mean:
> >> > >>> >> >
> >> > >>> >> >     Label l = Label("foo", "hello");
> >> > >>> >> > renders:
> >> > >>> >> >     <span wicket:id="foo">hello</span>
> >> > >>> >> >
> >> > >>> >> > ... some ajax stuff, or a normal page render:
> >> > >>> >> >
> >> > >>> >> >     l.setVisible(false);
> >> > >>> >> > renders:
> >> > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> >> > >>> >> > ?
> >> > >>> >> >
> >> > >>> >> > This can and will break existing clients in a very nasty
> >> manner,
> >> > >>> >> > because the markup id is still present in the final markup.
> >> > >>> >> >
> >> > >>> >> > Martijn
> >> > >>> >> >
> >> > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> >> wrote:
> >> > >>> >> > > Johan Compagner a écrit :
> >> > >>> >> > > >> > Also always just rendering the component but use the
> >> style
> >> > >>> >> to make in
> >> > >>> >> > > >> > invisible
> >> > >>> >> > > >> > could be a security problem. So that can't be the
> >> default.
> >> > >>> >> > > >>
> >> > >>> >> > > >> What do you mean by security problem?
> >> > >>> >> > > >
> >> > >>> >> > > >
> >> > >>> >> > > >
> >> > >>> >> > > > If the the component that is  set to none visible is
> none
> >> > >>> visible
> >> > >>> >> > > > because of
> >> > >>> >> > > > security
> >> > >>> >> > > > So it has data that never should be send to the browser
> >> > >>> because
> >> > >>> >> the
> >> > >>> >> > > > user is
> >> > >>> >> > > > not allowed
> >> > >>> >> > > > to see it.
> >> > >>> >> > >
> >> > >>> >> > > But data is never send to the user because a none visible
> >> > >>> >> component will
> >> > >>> >> > > be render as an empty tag, so without data
> >> > >>> >> > >
> >> > >>> >> > >
> >> > >>> >> >
> >> > >>> >> >
> >> > >>> >> > --
> >> > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> >> > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> >> > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket
> >> now!
> >> > >>> >> > http://wicketframework.org
> >> > >>> >> >
> >> > >>> >>
> >> > >>> >
> >> > >>> >
> >> > >>>
> >> > >>>
> >> > >>
> >> > >>
> >> > >
> >> > >
> >> >
> >> >
> >>
> >
>
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Igor Vaynberg a écrit :
> what you need is a different method. instead of 
> setoutputmarkupid(true) you
> need something that will do
>
> setoutputmarkuptag(boolean b) {
>  if (b) {
>     setoutputmarkupid(true);
>     setflag(outputmarkuptag,true);
>   } else {
>       setflag(outputmarkuptag,false);
>       //not sure if we should undo setoutputmarkupid here
>    }
> }

Yes, it is the current behavior of the patch ;) actually, the current 
behavior is
if outputMarkupid == true
    render componentTag + attribute style and id
else
    render nothing

so if you want to render nothing just set outputMarkupId to false, for 
the behavior we proposed just setoutputMarkupId to true

It is exactly what you describe in setoutputmarkuptag ;)

--
Vincent

>
> now this will solve the problems outlined so far
>
> btw the latest patch attached to the issue is bad because it executes
> behaviors which should not happen.
>
> what i am worried about are javascript libraries and css. does css 3 have
> support for odd/even selectors? that will break. this will even break 
> css 2
> first child selector if that happens to be invisible because i dont 
> think it
> checks for the display attribute. it will also break  3rd party 
> javascript
> libs that dont check for display attrs.
>
> i see how this makes life easier, but it also has a potential to be evil.
> that is why i closed the issue as wont-fix and told vincent to bring the
> discussion here on the list before we go any further.
>
> -igor
>
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>>
>> I don't like it. I don't think deprecating set/isVisible is the way to
>> go. Plus there are other reasons: As now we use one flag for visible
>> status. With your approach you'd require an enum, int, byte or
>> something similiar, that takes more space then just one bit in flags.
>>
>> I think we can just make a getter, that by default returns the value
>> from application settings. And you can override that for your
>> component, if you want something different that what's set in
>> application settings.
>>
>> -Matej
>>
>> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> > Frédéric Bertin a écrit :
>> > > Martijn Dashorst wrote:
>> > >>> it seems taht this kind of construction is used to make workaround
>> of
>> > >>> the bug. Is'n it?
>> > >>
>> > >> First, what bug? I don't know that this is a bug? I thought we are
>> > >> discussing a feature here. Secondly, this is not a workaround, but
>> > >> creating client side code based on a API contract: 
>> setVisible(false)
>> > >> removes the component markup completely, including its tags, 
>> from the
>> > >> final markup.
>> > > ok, then there's a need for 2 different mechanisms.
>> > > One to switch a component visibility. This one should be 
>> *reversible*
>> > > (in ajax or not), and then it should always output a tag with an id
>> > > attribute (actually, can be done only when setOutputMarkupId is 
>> set to
>> > > true).
>> > >
>> > > Another to render or not a component in the output markup. This one
>> > > should be documented as *not reversible*. This is the current
>> > > setVisible implementation.
>> > >
>> > > wdyt?
>> >
>> > +1
>> > What about keeping current behavior on setVisible (deprecated) and
>> > add a method setVisibility :
>> >     - none : render nothing
>> >     - visible : render all
>> >     - invisible : render only container tag with style:display-none
>> >
>> > Add in documentation
>> > none: can not become visible in ajax
>> > in visible: can
>> >
>> > I think it will match to all use case, no ?
>> > >
>> > >
>> > >
>> > >>
>> > >> It is based on the assumption that some element is *NOT* present at
>> > >> all. Your change will invert that behavior, and in such a way 
>> that it
>> > >> is only detectable by debugging your javascript. Not something I
>> > >> enjoy, nor 95% of the development community.
>> > >>
>> > >> You must understand that this is a major api break, not something
>> > >> minor. This is not detectable by a compiler. You *will* break
>> existing
>> > >> scripts, pages and applications in a non-obvious way. Silent 
>> failures
>> > >> are something we try to avoid at all cost.
>> > >>
>> > >> Martijn
>> > >>
>> > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> > >>> Martijn Dashorst a écrit :
>> > >>> > Currently everybody assumes (correctly) that the element is
>> > >>> completely
>> > >>> > removed (Ajax and non-Ajax)
>> > >>> Yes of course, but it is the same for all workarounds ;)
>> > >>> When to change servlet to filter, users have to change their web
>> xml.
>> > >>> Each time you change something users have to adapt their code
>> > >>> > i.e. not present in the final markup.
>> > >>> > This means that scripts that iterate through the dom, or 
>> check for
>> > >>> the
>> > >>> > document.getElementById() == null will fail if we implement 
>> this.
>> > >>> >
>> > >>> it seems taht this kind of construction is used to make workaround
>> of
>> > >>> the bug. Is'n it?
>> > >>> > I *strongly* discourage changing this behavior.
>> > >>> >
>> > >>> > Martijn
>> > >>> >
>> > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> > >>> >> Will it? This seems to be actually quite a smart workaround. 
>> How
>> > >>> >> exactly will this break existing clients? Only thing i'm
>> concerned
>> > >>> >> about is the validity of output markup. but imho when we 
>> preserve
>> > >>> the
>> > >>> >> original tag names, e.g. td will render as td, it should be all
>> > >>> right.
>> > >>> >>
>> > >>> >> -Matej
>> > >>> >>
>> > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> 
>> wrote:
>> > >>> >> > So you mean:
>> > >>> >> >
>> > >>> >> >     Label l = Label("foo", "hello");
>> > >>> >> > renders:
>> > >>> >> >     <span wicket:id="foo">hello</span>
>> > >>> >> >
>> > >>> >> > ... some ajax stuff, or a normal page render:
>> > >>> >> >
>> > >>> >> >     l.setVisible(false);
>> > >>> >> > renders:
>> > >>> >> >     <span wicket:id="foo" style="display:none"></span>
>> > >>> >> > ?
>> > >>> >> >
>> > >>> >> > This can and will break existing clients in a very nasty
>> manner,
>> > >>> >> > because the markup id is still present in the final markup.
>> > >>> >> >
>> > >>> >> > Martijn
>> > >>> >> >
>> > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
>> wrote:
>> > >>> >> > > Johan Compagner a écrit :
>> > >>> >> > > >> > Also always just rendering the component but use the
>> style
>> > >>> >> to make in
>> > >>> >> > > >> > invisible
>> > >>> >> > > >> > could be a security problem. So that can't be the
>> default.
>> > >>> >> > > >>
>> > >>> >> > > >> What do you mean by security problem?
>> > >>> >> > > >
>> > >>> >> > > >
>> > >>> >> > > >
>> > >>> >> > > > If the the component that is  set to none visible is none
>> > >>> visible
>> > >>> >> > > > because of
>> > >>> >> > > > security
>> > >>> >> > > > So it has data that never should be send to the browser
>> > >>> because
>> > >>> >> the
>> > >>> >> > > > user is
>> > >>> >> > > > not allowed
>> > >>> >> > > > to see it.
>> > >>> >> > >
>> > >>> >> > > But data is never send to the user because a none visible
>> > >>> >> component will
>> > >>> >> > > be render as an empty tag, so without data
>> > >>> >> > >
>> > >>> >> > >
>> > >>> >> >
>> > >>> >> >
>> > >>> >> > --
>> > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket 
>> now!
>> > >>> >> > http://wicketframework.org
>> > >>> >> >
>> > >>> >>
>> > >>> >
>> > >>> >
>> > >>>
>> > >>>
>> > >>
>> > >>
>> > >
>> > >
>> >
>> >
>>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Igor Vaynberg <ig...@gmail.com>.
what you need is a different method. instead of setoutputmarkupid(true) you
need something that will do

setoutputmarkuptag(boolean b) {
  if (b) {
     setoutputmarkupid(true);
     setflag(outputmarkuptag,true);
   } else {
       setflag(outputmarkuptag,false);
       //not sure if we should undo setoutputmarkupid here
    }
}

now this will solve the problems outlined so far

btw the latest patch attached to the issue is bad because it executes
behaviors which should not happen.

what i am worried about are javascript libraries and css. does css 3 have
support for odd/even selectors? that will break. this will even break css 2
first child selector if that happens to be invisible because i dont think it
checks for the display attribute. it will also break  3rd party javascript
libs that dont check for display attrs.

i see how this makes life easier, but it also has a potential to be evil.
that is why i closed the issue as wont-fix and told vincent to bring the
discussion here on the list before we go any further.

-igor


On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>
> I don't like it. I don't think deprecating set/isVisible is the way to
> go. Plus there are other reasons: As now we use one flag for visible
> status. With your approach you'd require an enum, int, byte or
> something similiar, that takes more space then just one bit in flags.
>
> I think we can just make a getter, that by default returns the value
> from application settings. And you can override that for your
> component, if you want something different that what's set in
> application settings.
>
> -Matej
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > Frédéric Bertin a écrit :
> > > Martijn Dashorst wrote:
> > >>> it seems taht this kind of construction is used to make workaround
> of
> > >>> the bug. Is'n it?
> > >>
> > >> First, what bug? I don't know that this is a bug? I thought we are
> > >> discussing a feature here. Secondly, this is not a workaround, but
> > >> creating client side code based on a API contract: setVisible(false)
> > >> removes the component markup completely, including its tags, from the
> > >> final markup.
> > > ok, then there's a need for 2 different mechanisms.
> > > One to switch a component visibility. This one should be *reversible*
> > > (in ajax or not), and then it should always output a tag with an id
> > > attribute (actually, can be done only when setOutputMarkupId is set to
> > > true).
> > >
> > > Another to render or not a component in the output markup. This one
> > > should be documented as *not reversible*. This is the current
> > > setVisible implementation.
> > >
> > > wdyt?
> >
> > +1
> > What about keeping current behavior on setVisible (deprecated) and
> > add a method setVisibility :
> >     - none : render nothing
> >     - visible : render all
> >     - invisible : render only container tag with style:display-none
> >
> > Add in documentation
> > none: can not become visible in ajax
> > in visible: can
> >
> > I think it will match to all use case, no ?
> > >
> > >
> > >
> > >>
> > >> It is based on the assumption that some element is *NOT* present at
> > >> all. Your change will invert that behavior, and in such a way that it
> > >> is only detectable by debugging your javascript. Not something I
> > >> enjoy, nor 95% of the development community.
> > >>
> > >> You must understand that this is a major api break, not something
> > >> minor. This is not detectable by a compiler. You *will* break
> existing
> > >> scripts, pages and applications in a non-obvious way. Silent failures
> > >> are something we try to avoid at all cost.
> > >>
> > >> Martijn
> > >>
> > >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > >>> Martijn Dashorst a écrit :
> > >>> > Currently everybody assumes (correctly) that the element is
> > >>> completely
> > >>> > removed (Ajax and non-Ajax)
> > >>> Yes of course, but it is the same for all workarounds ;)
> > >>> When to change servlet to filter, users have to change their web
> xml.
> > >>> Each time you change something users have to adapt their code
> > >>> > i.e. not present in the final markup.
> > >>> > This means that scripts that iterate through the dom, or check for
> > >>> the
> > >>> > document.getElementById() == null will fail if we implement this.
> > >>> >
> > >>> it seems taht this kind of construction is used to make workaround
> of
> > >>> the bug. Is'n it?
> > >>> > I *strongly* discourage changing this behavior.
> > >>> >
> > >>> > Martijn
> > >>> >
> > >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > >>> >> Will it? This seems to be actually quite a smart workaround. How
> > >>> >> exactly will this break existing clients? Only thing i'm
> concerned
> > >>> >> about is the validity of output markup. but imho when we preserve
> > >>> the
> > >>> >> original tag names, e.g. td will render as td, it should be all
> > >>> right.
> > >>> >>
> > >>> >> -Matej
> > >>> >>
> > >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > >>> >> > So you mean:
> > >>> >> >
> > >>> >> >     Label l = Label("foo", "hello");
> > >>> >> > renders:
> > >>> >> >     <span wicket:id="foo">hello</span>
> > >>> >> >
> > >>> >> > ... some ajax stuff, or a normal page render:
> > >>> >> >
> > >>> >> >     l.setVisible(false);
> > >>> >> > renders:
> > >>> >> >     <span wicket:id="foo" style="display:none"></span>
> > >>> >> > ?
> > >>> >> >
> > >>> >> > This can and will break existing clients in a very nasty
> manner,
> > >>> >> > because the markup id is still present in the final markup.
> > >>> >> >
> > >>> >> > Martijn
> > >>> >> >
> > >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com>
> wrote:
> > >>> >> > > Johan Compagner a écrit :
> > >>> >> > > >> > Also always just rendering the component but use the
> style
> > >>> >> to make in
> > >>> >> > > >> > invisible
> > >>> >> > > >> > could be a security problem. So that can't be the
> default.
> > >>> >> > > >>
> > >>> >> > > >> What do you mean by security problem?
> > >>> >> > > >
> > >>> >> > > >
> > >>> >> > > >
> > >>> >> > > > If the the component that is  set to none visible is none
> > >>> visible
> > >>> >> > > > because of
> > >>> >> > > > security
> > >>> >> > > > So it has data that never should be send to the browser
> > >>> because
> > >>> >> the
> > >>> >> > > > user is
> > >>> >> > > > not allowed
> > >>> >> > > > to see it.
> > >>> >> > >
> > >>> >> > > But data is never send to the user because a none visible
> > >>> >> component will
> > >>> >> > > be render as an empty tag, so without data
> > >>> >> > >
> > >>> >> > >
> > >>> >> >
> > >>> >> >
> > >>> >> > --
> > >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> > >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > >>> >> > http://wicketframework.org
> > >>> >> >
> > >>> >>
> > >>> >
> > >>> >
> > >>>
> > >>>
> > >>
> > >>
> > >
> > >
> >
> >
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
I don't like it. I don't think deprecating set/isVisible is the way to
go. Plus there are other reasons: As now we use one flag for visible
status. With your approach you'd require an enum, int, byte or
something similiar, that takes more space then just one bit in flags.

I think we can just make a getter, that by default returns the value
from application settings. And you can override that for your
component, if you want something different that what's set in
application settings.

-Matej

On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> Frédéric Bertin a écrit :
> > Martijn Dashorst wrote:
> >>> it seems taht this kind of construction is used to make workaround of
> >>> the bug. Is'n it?
> >>
> >> First, what bug? I don't know that this is a bug? I thought we are
> >> discussing a feature here. Secondly, this is not a workaround, but
> >> creating client side code based on a API contract: setVisible(false)
> >> removes the component markup completely, including its tags, from the
> >> final markup.
> > ok, then there's a need for 2 different mechanisms.
> > One to switch a component visibility. This one should be *reversible*
> > (in ajax or not), and then it should always output a tag with an id
> > attribute (actually, can be done only when setOutputMarkupId is set to
> > true).
> >
> > Another to render or not a component in the output markup. This one
> > should be documented as *not reversible*. This is the current
> > setVisible implementation.
> >
> > wdyt?
>
> +1
> What about keeping current behavior on setVisible (deprecated) and
> add a method setVisibility :
>     - none : render nothing
>     - visible : render all
>     - invisible : render only container tag with style:display-none
>
> Add in documentation
> none: can not become visible in ajax
> in visible: can
>
> I think it will match to all use case, no ?
> >
> >
> >
> >>
> >> It is based on the assumption that some element is *NOT* present at
> >> all. Your change will invert that behavior, and in such a way that it
> >> is only detectable by debugging your javascript. Not something I
> >> enjoy, nor 95% of the development community.
> >>
> >> You must understand that this is a major api break, not something
> >> minor. This is not detectable by a compiler. You *will* break existing
> >> scripts, pages and applications in a non-obvious way. Silent failures
> >> are something we try to avoid at all cost.
> >>
> >> Martijn
> >>
> >> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >>> Martijn Dashorst a écrit :
> >>> > Currently everybody assumes (correctly) that the element is
> >>> completely
> >>> > removed (Ajax and non-Ajax)
> >>> Yes of course, but it is the same for all workarounds ;)
> >>> When to change servlet to filter, users have to change their web xml.
> >>> Each time you change something users have to adapt their code
> >>> > i.e. not present in the final markup.
> >>> > This means that scripts that iterate through the dom, or check for
> >>> the
> >>> > document.getElementById() == null will fail if we implement this.
> >>> >
> >>> it seems taht this kind of construction is used to make workaround of
> >>> the bug. Is'n it?
> >>> > I *strongly* discourage changing this behavior.
> >>> >
> >>> > Martijn
> >>> >
> >>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >>> >> Will it? This seems to be actually quite a smart workaround. How
> >>> >> exactly will this break existing clients? Only thing i'm concerned
> >>> >> about is the validity of output markup. but imho when we preserve
> >>> the
> >>> >> original tag names, e.g. td will render as td, it should be all
> >>> right.
> >>> >>
> >>> >> -Matej
> >>> >>
> >>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> >>> >> > So you mean:
> >>> >> >
> >>> >> >     Label l = Label("foo", "hello");
> >>> >> > renders:
> >>> >> >     <span wicket:id="foo">hello</span>
> >>> >> >
> >>> >> > ... some ajax stuff, or a normal page render:
> >>> >> >
> >>> >> >     l.setVisible(false);
> >>> >> > renders:
> >>> >> >     <span wicket:id="foo" style="display:none"></span>
> >>> >> > ?
> >>> >> >
> >>> >> > This can and will break existing clients in a very nasty manner,
> >>> >> > because the markup id is still present in the final markup.
> >>> >> >
> >>> >> > Martijn
> >>> >> >
> >>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >>> >> > > Johan Compagner a écrit :
> >>> >> > > >> > Also always just rendering the component but use the style
> >>> >> to make in
> >>> >> > > >> > invisible
> >>> >> > > >> > could be a security problem. So that can't be the default.
> >>> >> > > >>
> >>> >> > > >> What do you mean by security problem?
> >>> >> > > >
> >>> >> > > >
> >>> >> > > >
> >>> >> > > > If the the component that is  set to none visible is none
> >>> visible
> >>> >> > > > because of
> >>> >> > > > security
> >>> >> > > > So it has data that never should be send to the browser
> >>> because
> >>> >> the
> >>> >> > > > user is
> >>> >> > > > not allowed
> >>> >> > > > to see it.
> >>> >> > >
> >>> >> > > But data is never send to the user because a none visible
> >>> >> component will
> >>> >> > > be render as an empty tag, so without data
> >>> >> > >
> >>> >> > >
> >>> >> >
> >>> >> >
> >>> >> > --
> >>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> >>> >> > Join the wicket community at irc.freenode.net: ##wicket
> >>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> >>> >> > http://wicketframework.org
> >>> >> >
> >>> >>
> >>> >
> >>> >
> >>>
> >>>
> >>
> >>
> >
> >
>
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Frédéric Bertin a écrit :
> Martijn Dashorst wrote:
>>> it seems taht this kind of construction is used to make workaround of
>>> the bug. Is'n it?
>>
>> First, what bug? I don't know that this is a bug? I thought we are
>> discussing a feature here. Secondly, this is not a workaround, but
>> creating client side code based on a API contract: setVisible(false)
>> removes the component markup completely, including its tags, from the
>> final markup.
> ok, then there's a need for 2 different mechanisms.
> One to switch a component visibility. This one should be *reversible* 
> (in ajax or not), and then it should always output a tag with an id 
> attribute (actually, can be done only when setOutputMarkupId is set to 
> true).
>
> Another to render or not a component in the output markup. This one 
> should be documented as *not reversible*. This is the current 
> setVisible implementation.
>
> wdyt?

+1
What about keeping current behavior on setVisible (deprecated) and
add a method setVisibility :
    - none : render nothing
    - visible : render all
    - invisible : render only container tag with style:display-none

Add in documentation
none: can not become visible in ajax
in visible: can

I think it will match to all use case, no ?
>
>
>
>>
>> It is based on the assumption that some element is *NOT* present at
>> all. Your change will invert that behavior, and in such a way that it
>> is only detectable by debugging your javascript. Not something I
>> enjoy, nor 95% of the development community.
>>
>> You must understand that this is a major api break, not something
>> minor. This is not detectable by a compiler. You *will* break existing
>> scripts, pages and applications in a non-obvious way. Silent failures
>> are something we try to avoid at all cost.
>>
>> Martijn
>>
>> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>>> Martijn Dashorst a écrit :
>>> > Currently everybody assumes (correctly) that the element is 
>>> completely
>>> > removed (Ajax and non-Ajax)
>>> Yes of course, but it is the same for all workarounds ;)
>>> When to change servlet to filter, users have to change their web xml.
>>> Each time you change something users have to adapt their code
>>> > i.e. not present in the final markup.
>>> > This means that scripts that iterate through the dom, or check for 
>>> the
>>> > document.getElementById() == null will fail if we implement this.
>>> >
>>> it seems taht this kind of construction is used to make workaround of
>>> the bug. Is'n it?
>>> > I *strongly* discourage changing this behavior.
>>> >
>>> > Martijn
>>> >
>>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>>> >> Will it? This seems to be actually quite a smart workaround. How
>>> >> exactly will this break existing clients? Only thing i'm concerned
>>> >> about is the validity of output markup. but imho when we preserve 
>>> the
>>> >> original tag names, e.g. td will render as td, it should be all 
>>> right.
>>> >>
>>> >> -Matej
>>> >>
>>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
>>> >> > So you mean:
>>> >> >
>>> >> >     Label l = Label("foo", "hello");
>>> >> > renders:
>>> >> >     <span wicket:id="foo">hello</span>
>>> >> >
>>> >> > ... some ajax stuff, or a normal page render:
>>> >> >
>>> >> >     l.setVisible(false);
>>> >> > renders:
>>> >> >     <span wicket:id="foo" style="display:none"></span>
>>> >> > ?
>>> >> >
>>> >> > This can and will break existing clients in a very nasty manner,
>>> >> > because the markup id is still present in the final markup.
>>> >> >
>>> >> > Martijn
>>> >> >
>>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>>> >> > > Johan Compagner a écrit :
>>> >> > > >> > Also always just rendering the component but use the style
>>> >> to make in
>>> >> > > >> > invisible
>>> >> > > >> > could be a security problem. So that can't be the default.
>>> >> > > >>
>>> >> > > >> What do you mean by security problem?
>>> >> > > >
>>> >> > > >
>>> >> > > >
>>> >> > > > If the the component that is  set to none visible is none 
>>> visible
>>> >> > > > because of
>>> >> > > > security
>>> >> > > > So it has data that never should be send to the browser 
>>> because
>>> >> the
>>> >> > > > user is
>>> >> > > > not allowed
>>> >> > > > to see it.
>>> >> > >
>>> >> > > But data is never send to the user because a none visible
>>> >> component will
>>> >> > > be render as an empty tag, so without data
>>> >> > >
>>> >> > >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>>> >> > Join the wicket community at irc.freenode.net: ##wicket
>>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
>>> >> > http://wicketframework.org
>>> >> >
>>> >>
>>> >
>>> >
>>>
>>>
>>
>>
>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Frédéric Bertin <fr...@anyware-tech.com>.
Martijn Dashorst wrote:
>> it seems taht this kind of construction is used to make workaround of
>> the bug. Is'n it?
>
> First, what bug? I don't know that this is a bug? I thought we are
> discussing a feature here. Secondly, this is not a workaround, but
> creating client side code based on a API contract: setVisible(false)
> removes the component markup completely, including its tags, from the
> final markup.
ok, then there's a need for 2 different mechanisms.
One to switch a component visibility. This one should be *reversible* 
(in ajax or not), and then it should always output a tag with an id 
attribute (actually, can be done only when setOutputMarkupId is set to 
true).

Another to render or not a component in the output markup. This one 
should be documented as *not reversible*. This is the current setVisible 
implementation.

wdyt?



>
> It is based on the assumption that some element is *NOT* present at
> all. Your change will invert that behavior, and in such a way that it
> is only detectable by debugging your javascript. Not something I
> enjoy, nor 95% of the development community.
>
> You must understand that this is a major api break, not something
> minor. This is not detectable by a compiler. You *will* break existing
> scripts, pages and applications in a non-obvious way. Silent failures
> are something we try to avoid at all cost.
>
> Martijn
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> Martijn Dashorst a écrit :
>> > Currently everybody assumes (correctly) that the element is completely
>> > removed (Ajax and non-Ajax)
>> Yes of course, but it is the same for all workarounds ;)
>> When to change servlet to filter, users have to change their web xml.
>> Each time you change something users have to adapt their code
>> > i.e. not present in the final markup.
>> > This means that scripts that iterate through the dom, or check for the
>> > document.getElementById() == null will fail if we implement this.
>> >
>> it seems taht this kind of construction is used to make workaround of
>> the bug. Is'n it?
>> > I *strongly* discourage changing this behavior.
>> >
>> > Martijn
>> >
>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >> Will it? This seems to be actually quite a smart workaround. How
>> >> exactly will this break existing clients? Only thing i'm concerned
>> >> about is the validity of output markup. but imho when we preserve the
>> >> original tag names, e.g. td will render as td, it should be all 
>> right.
>> >>
>> >> -Matej
>> >>
>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
>> >> > So you mean:
>> >> >
>> >> >     Label l = Label("foo", "hello");
>> >> > renders:
>> >> >     <span wicket:id="foo">hello</span>
>> >> >
>> >> > ... some ajax stuff, or a normal page render:
>> >> >
>> >> >     l.setVisible(false);
>> >> > renders:
>> >> >     <span wicket:id="foo" style="display:none"></span>
>> >> > ?
>> >> >
>> >> > This can and will break existing clients in a very nasty manner,
>> >> > because the markup id is still present in the final markup.
>> >> >
>> >> > Martijn
>> >> >
>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> >> > > Johan Compagner a écrit :
>> >> > > >> > Also always just rendering the component but use the style
>> >> to make in
>> >> > > >> > invisible
>> >> > > >> > could be a security problem. So that can't be the default.
>> >> > > >>
>> >> > > >> What do you mean by security problem?
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > > > If the the component that is  set to none visible is none 
>> visible
>> >> > > > because of
>> >> > > > security
>> >> > > > So it has data that never should be send to the browser because
>> >> the
>> >> > > > user is
>> >> > > > not allowed
>> >> > > > to see it.
>> >> > >
>> >> > > But data is never send to the user because a none visible
>> >> component will
>> >> > > be render as an empty tag, so without data
>> >> > >
>> >> > >
>> >> >
>> >> >
>> >> > --
>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
>> >> > http://wicketframework.org
>> >> >
>> >>
>> >
>> >
>>
>>
>
>


-- 
Frédéric BERTIN
Contrôle industriel
ANYWARE TECHNOLOGIES
Tél (direct) : +33 (0)5.61.00.06.41
Tél (standard) : +33 (0)5.61.00.52.90
Fax : +33 (0)5 61 00 51 46
http://www.anyware-tech.com


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Martijn Dashorst a écrit :
>> it seems taht this kind of construction is used to make workaround of
>> the bug. Is'n it?
>
> First, what bug? I don't know that this is a bug? I thought we are
> discussing a feature here. Secondly, this is not a workaround, but
> creating client side code based on a API contract: setVisible(false)
> removes the component markup completely, including its tags, from the
> final markup.
>
> It is based on the assumption that some element is *NOT* present at
> all. Your change will invert that behavior, and in such a way that it
> is only detectable by debugging your javascript. Not something I
> enjoy, nor 95% of the development community.
>
> You must understand that this is a major api break, not something
> minor. This is not detectable by a compiler. You *will* break existing
> scripts, pages and applications in a non-obvious way. Silent failures
> are something we try to avoid at all cost.
Yes and it is very wise.
But I think there is a migration guide with each release. So you just 
have to add this change in it.

>
> Martijn
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> Martijn Dashorst a écrit :
>> > Currently everybody assumes (correctly) that the element is completely
>> > removed (Ajax and non-Ajax)
>> Yes of course, but it is the same for all workarounds ;)
>> When to change servlet to filter, users have to change their web xml.
>> Each time you change something users have to adapt their code
>> > i.e. not present in the final markup.
>> > This means that scripts that iterate through the dom, or check for the
>> > document.getElementById() == null will fail if we implement this.
>> >
>> it seems taht this kind of construction is used to make workaround of
>> the bug. Is'n it?
>> > I *strongly* discourage changing this behavior.
>> >
>> > Martijn
>> >
>> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> >> Will it? This seems to be actually quite a smart workaround. How
>> >> exactly will this break existing clients? Only thing i'm concerned
>> >> about is the validity of output markup. but imho when we preserve the
>> >> original tag names, e.g. td will render as td, it should be all 
>> right.
>> >>
>> >> -Matej
>> >>
>> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
>> >> > So you mean:
>> >> >
>> >> >     Label l = Label("foo", "hello");
>> >> > renders:
>> >> >     <span wicket:id="foo">hello</span>
>> >> >
>> >> > ... some ajax stuff, or a normal page render:
>> >> >
>> >> >     l.setVisible(false);
>> >> > renders:
>> >> >     <span wicket:id="foo" style="display:none"></span>
>> >> > ?
>> >> >
>> >> > This can and will break existing clients in a very nasty manner,
>> >> > because the markup id is still present in the final markup.
>> >> >
>> >> > Martijn
>> >> >
>> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> >> > > Johan Compagner a écrit :
>> >> > > >> > Also always just rendering the component but use the style
>> >> to make in
>> >> > > >> > invisible
>> >> > > >> > could be a security problem. So that can't be the default.
>> >> > > >>
>> >> > > >> What do you mean by security problem?
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > > > If the the component that is  set to none visible is none 
>> visible
>> >> > > > because of
>> >> > > > security
>> >> > > > So it has data that never should be send to the browser because
>> >> the
>> >> > > > user is
>> >> > > > not allowed
>> >> > > > to see it.
>> >> > >
>> >> > > But data is never send to the user because a none visible
>> >> component will
>> >> > > be render as an empty tag, so without data
>> >> > >
>> >> > >
>> >> >
>> >> >
>> >> > --
>> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> >> > Join the wicket community at irc.freenode.net: ##wicket
>> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
>> >> > http://wicketframework.org
>> >> >
>> >>
>> >
>> >
>>
>>
>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Johan Compagner <jc...@gmail.com>.
WebMarkupContainer isn't the right choice i think
Then we miss Label (which is a WebComponent)

johan


On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>
> I don't think current behavior is the best one. Even if it can be
> "corrent", it lacks convenience.
>
> I suggest a flag on web markup container, that could change the way
> invisible component is rendered. Default could be, that it's not
> rendered at all. So it would not break any code. You could change it
> though, to render invisible component as <componentTag
> style="display:none" id="..."></componentTag>. This would work for
> both ajax and regular renders, so that you could render invisible
> component and later make it visible with ajax request.
>
> -Matej
>
> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > it seems taht this kind of construction is used to make workaround of
> > > the bug. Is'n it?
> >
> > First, what bug? I don't know that this is a bug? I thought we are
> > discussing a feature here. Secondly, this is not a workaround, but
> > creating client side code based on a API contract: setVisible(false)
> > removes the component markup completely, including its tags, from the
> > final markup.
> >
> > It is based on the assumption that some element is *NOT* present at
> > all. Your change will invert that behavior, and in such a way that it
> > is only detectable by debugging your javascript. Not something I
> > enjoy, nor 95% of the development community.
> >
> > You must understand that this is a major api break, not something
> > minor. This is not detectable by a compiler. You *will* break existing
> > scripts, pages and applications in a non-obvious way. Silent failures
> > are something we try to avoid at all cost.
> >
> > Martijn
> >
> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > Martijn Dashorst a écrit :
> > > > Currently everybody assumes (correctly) that the element is
> completely
> > > > removed (Ajax and non-Ajax)
> > > Yes of course, but it is the same for all workarounds ;)
> > > When to change servlet to filter, users have to change their web xml.
> > > Each time you change something users have to adapt their code
> > > > i.e. not present in the final markup.
> > > > This means that scripts that iterate through the dom, or check for
> the
> > > > document.getElementById() == null will fail if we implement this.
> > > >
> > > it seems taht this kind of construction is used to make workaround of
> > > the bug. Is'n it?
> > > > I *strongly* discourage changing this behavior.
> > > >
> > > > Martijn
> > > >
> > > > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > > >> Will it? This seems to be actually quite a smart workaround. How
> > > >> exactly will this break existing clients? Only thing i'm concerned
> > > >> about is the validity of output markup. but imho when we preserve
> the
> > > >> original tag names, e.g. td will render as td, it should be all
> right.
> > > >>
> > > >> -Matej
> > > >>
> > > >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > > >> > So you mean:
> > > >> >
> > > >> >     Label l = Label("foo", "hello");
> > > >> > renders:
> > > >> >     <span wicket:id="foo">hello</span>
> > > >> >
> > > >> > ... some ajax stuff, or a normal page render:
> > > >> >
> > > >> >     l.setVisible(false);
> > > >> > renders:
> > > >> >     <span wicket:id="foo" style="display:none"></span>
> > > >> > ?
> > > >> >
> > > >> > This can and will break existing clients in a very nasty manner,
> > > >> > because the markup id is still present in the final markup.
> > > >> >
> > > >> > Martijn
> > > >> >
> > > >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > >> > > Johan Compagner a écrit :
> > > >> > > >> > Also always just rendering the component but use the style
> > > >> to make in
> > > >> > > >> > invisible
> > > >> > > >> > could be a security problem. So that can't be the default.
> > > >> > > >>
> > > >> > > >> What do you mean by security problem?
> > > >> > > >
> > > >> > > >
> > > >> > > >
> > > >> > > > If the the component that is  set to none visible is none
> visible
> > > >> > > > because of
> > > >> > > > security
> > > >> > > > So it has data that never should be send to the browser
> because
> > > >> the
> > > >> > > > user is
> > > >> > > > not allowed
> > > >> > > > to see it.
> > > >> > >
> > > >> > > But data is never send to the user because a none visible
> > > >> component will
> > > >> > > be render as an empty tag, so without data
> > > >> > >
> > > >> > >
> > > >> >
> > > >> >
> > > >> > --
> > > >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > > >> > Join the wicket community at irc.freenode.net: ##wicket
> > > >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > > >> > http://wicketframework.org
> > > >> >
> > > >>
> > > >
> > > >
> > >
> > >
> >
> >
> > --
> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > Join the wicket community at irc.freenode.net: ##wicket
> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > http://wicketframework.org
> >
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
I don't think current behavior is the best one. Even if it can be
"corrent", it lacks convenience.

I suggest a flag on web markup container, that could change the way
invisible component is rendered. Default could be, that it's not
rendered at all. So it would not break any code. You could change it
though, to render invisible component as <componentTag
style="display:none" id="..."></componentTag>. This would work for
both ajax and regular renders, so that you could render invisible
component and later make it visible with ajax request.

-Matej

On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > it seems taht this kind of construction is used to make workaround of
> > the bug. Is'n it?
>
> First, what bug? I don't know that this is a bug? I thought we are
> discussing a feature here. Secondly, this is not a workaround, but
> creating client side code based on a API contract: setVisible(false)
> removes the component markup completely, including its tags, from the
> final markup.
>
> It is based on the assumption that some element is *NOT* present at
> all. Your change will invert that behavior, and in such a way that it
> is only detectable by debugging your javascript. Not something I
> enjoy, nor 95% of the development community.
>
> You must understand that this is a major api break, not something
> minor. This is not detectable by a compiler. You *will* break existing
> scripts, pages and applications in a non-obvious way. Silent failures
> are something we try to avoid at all cost.
>
> Martijn
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > Martijn Dashorst a écrit :
> > > Currently everybody assumes (correctly) that the element is completely
> > > removed (Ajax and non-Ajax)
> > Yes of course, but it is the same for all workarounds ;)
> > When to change servlet to filter, users have to change their web xml.
> > Each time you change something users have to adapt their code
> > > i.e. not present in the final markup.
> > > This means that scripts that iterate through the dom, or check for the
> > > document.getElementById() == null will fail if we implement this.
> > >
> > it seems taht this kind of construction is used to make workaround of
> > the bug. Is'n it?
> > > I *strongly* discourage changing this behavior.
> > >
> > > Martijn
> > >
> > > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> > >> Will it? This seems to be actually quite a smart workaround. How
> > >> exactly will this break existing clients? Only thing i'm concerned
> > >> about is the validity of output markup. but imho when we preserve the
> > >> original tag names, e.g. td will render as td, it should be all right.
> > >>
> > >> -Matej
> > >>
> > >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > >> > So you mean:
> > >> >
> > >> >     Label l = Label("foo", "hello");
> > >> > renders:
> > >> >     <span wicket:id="foo">hello</span>
> > >> >
> > >> > ... some ajax stuff, or a normal page render:
> > >> >
> > >> >     l.setVisible(false);
> > >> > renders:
> > >> >     <span wicket:id="foo" style="display:none"></span>
> > >> > ?
> > >> >
> > >> > This can and will break existing clients in a very nasty manner,
> > >> > because the markup id is still present in the final markup.
> > >> >
> > >> > Martijn
> > >> >
> > >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > >> > > Johan Compagner a écrit :
> > >> > > >> > Also always just rendering the component but use the style
> > >> to make in
> > >> > > >> > invisible
> > >> > > >> > could be a security problem. So that can't be the default.
> > >> > > >>
> > >> > > >> What do you mean by security problem?
> > >> > > >
> > >> > > >
> > >> > > >
> > >> > > > If the the component that is  set to none visible is none visible
> > >> > > > because of
> > >> > > > security
> > >> > > > So it has data that never should be send to the browser because
> > >> the
> > >> > > > user is
> > >> > > > not allowed
> > >> > > > to see it.
> > >> > >
> > >> > > But data is never send to the user because a none visible
> > >> component will
> > >> > > be render as an empty tag, so without data
> > >> > >
> > >> > >
> > >> >
> > >> >
> > >> > --
> > >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > >> > Join the wicket community at irc.freenode.net: ##wicket
> > >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > >> > http://wicketframework.org
> > >> >
> > >>
> > >
> > >
> >
> >
>
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
> it seems taht this kind of construction is used to make workaround of
> the bug. Is'n it?

First, what bug? I don't know that this is a bug? I thought we are
discussing a feature here. Secondly, this is not a workaround, but
creating client side code based on a API contract: setVisible(false)
removes the component markup completely, including its tags, from the
final markup.

It is based on the assumption that some element is *NOT* present at
all. Your change will invert that behavior, and in such a way that it
is only detectable by debugging your javascript. Not something I
enjoy, nor 95% of the development community.

You must understand that this is a major api break, not something
minor. This is not detectable by a compiler. You *will* break existing
scripts, pages and applications in a non-obvious way. Silent failures
are something we try to avoid at all cost.

Martijn

On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> Martijn Dashorst a écrit :
> > Currently everybody assumes (correctly) that the element is completely
> > removed (Ajax and non-Ajax)
> Yes of course, but it is the same for all workarounds ;)
> When to change servlet to filter, users have to change their web xml.
> Each time you change something users have to adapt their code
> > i.e. not present in the final markup.
> > This means that scripts that iterate through the dom, or check for the
> > document.getElementById() == null will fail if we implement this.
> >
> it seems taht this kind of construction is used to make workaround of
> the bug. Is'n it?
> > I *strongly* discourage changing this behavior.
> >
> > Martijn
> >
> > On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> >> Will it? This seems to be actually quite a smart workaround. How
> >> exactly will this break existing clients? Only thing i'm concerned
> >> about is the validity of output markup. but imho when we preserve the
> >> original tag names, e.g. td will render as td, it should be all right.
> >>
> >> -Matej
> >>
> >> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> >> > So you mean:
> >> >
> >> >     Label l = Label("foo", "hello");
> >> > renders:
> >> >     <span wicket:id="foo">hello</span>
> >> >
> >> > ... some ajax stuff, or a normal page render:
> >> >
> >> >     l.setVisible(false);
> >> > renders:
> >> >     <span wicket:id="foo" style="display:none"></span>
> >> > ?
> >> >
> >> > This can and will break existing clients in a very nasty manner,
> >> > because the markup id is still present in the final markup.
> >> >
> >> > Martijn
> >> >
> >> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> >> > > Johan Compagner a écrit :
> >> > > >> > Also always just rendering the component but use the style
> >> to make in
> >> > > >> > invisible
> >> > > >> > could be a security problem. So that can't be the default.
> >> > > >>
> >> > > >> What do you mean by security problem?
> >> > > >
> >> > > >
> >> > > >
> >> > > > If the the component that is  set to none visible is none visible
> >> > > > because of
> >> > > > security
> >> > > > So it has data that never should be send to the browser because
> >> the
> >> > > > user is
> >> > > > not allowed
> >> > > > to see it.
> >> > >
> >> > > But data is never send to the user because a none visible
> >> component will
> >> > > be render as an empty tag, so without data
> >> > >
> >> > >
> >> >
> >> >
> >> > --
> >> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> >> > Join the wicket community at irc.freenode.net: ##wicket
> >> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> >> > http://wicketframework.org
> >> >
> >>
> >
> >
>
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Martijn Dashorst a écrit :
> Currently everybody assumes (correctly) that the element is completely
> removed (Ajax and non-Ajax)
Yes of course, but it is the same for all workarounds ;)
When to change servlet to filter, users have to change their web xml.
Each time you change something users have to adapt their code
> i.e. not present in the final markup.
> This means that scripts that iterate through the dom, or check for the
> document.getElementById() == null will fail if we implement this.
>
it seems taht this kind of construction is used to make workaround of 
the bug. Is'n it?
> I *strongly* discourage changing this behavior.
>
> Martijn
>
> On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
>> Will it? This seems to be actually quite a smart workaround. How
>> exactly will this break existing clients? Only thing i'm concerned
>> about is the validity of output markup. but imho when we preserve the
>> original tag names, e.g. td will render as td, it should be all right.
>>
>> -Matej
>>
>> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
>> > So you mean:
>> >
>> >     Label l = Label("foo", "hello");
>> > renders:
>> >     <span wicket:id="foo">hello</span>
>> >
>> > ... some ajax stuff, or a normal page render:
>> >
>> >     l.setVisible(false);
>> > renders:
>> >     <span wicket:id="foo" style="display:none"></span>
>> > ?
>> >
>> > This can and will break existing clients in a very nasty manner,
>> > because the markup id is still present in the final markup.
>> >
>> > Martijn
>> >
>> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>> > > Johan Compagner a écrit :
>> > > >> > Also always just rendering the component but use the style 
>> to make in
>> > > >> > invisible
>> > > >> > could be a security problem. So that can't be the default.
>> > > >>
>> > > >> What do you mean by security problem?
>> > > >
>> > > >
>> > > >
>> > > > If the the component that is  set to none visible is none visible
>> > > > because of
>> > > > security
>> > > > So it has data that never should be send to the browser because 
>> the
>> > > > user is
>> > > > not allowed
>> > > > to see it.
>> > >
>> > > But data is never send to the user because a none visible 
>> component will
>> > > be render as an empty tag, so without data
>> > >
>> > >
>> >
>> >
>> > --
>> > Learn Wicket at ApacheCon Europe: http://apachecon.com
>> > Join the wicket community at irc.freenode.net: ##wicket
>> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
>> > http://wicketframework.org
>> >
>>
>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Frédéric Bertin <fr...@anyware-tech.com>.
Martijn Dashorst wrote:
> It is not a bug, but the current way how markup rendering works. It
> has been this way since 0.1. It is widely published behavior and
> widely used. Introducing this will create bugs in existing
> applications, that are hard to track.
>
> And please, don't give me JavaScript lessons, thank you very much, I
> find that very condescending. 
I'm sorry, it really wasn't my intention. What I tried to point out by 
writing this is the semantic difference between visibility and rendering.
The more I think about this, the more I find that the solution is 
similar to the distinction made in CSS between visibility 
(hidden|visible) and display (none|block).
What is currently implemented in Wicket using setVisible is similar to 
display (none|block), because the component is not rendered in the 
markup at all. And it is implemented in a non-reversible way.
What I need is more something similar to visibility(hidden|visible), 
because the component needs to be rendered in the markup (not 
completely, a tag with an @id is enough). And the implementation needs 
to be reversible.

wdyt?

> I understand you find this solution very
> elegant and perfect for *YOUR* usecase. As a framework
> builder/maintainer I have to weigh existing investments as well. I
> don't take breaking existing applications lightly.
>
> *IF* we were to adopt this, it should not be the default (because it
> breaks existing applications), and it should be an application setting
> to turn it on, or a page setting (which inherits the application
> setting?). I think that making it a WMC specific setting will mitigate
> the advantages of this approach.
>
> As for the migration guides, these are usually properly ignored. If
> you can fail fast, then we should do that. Typically we use changes in
> API and @deprecation for that. This change doesn't have any of those
> safeguards.
>
> Martijn
>
>
> On 3/19/07, Frédéric Bertin <fr...@anyware-tech.com> wrote:
>> Martijn Dashorst wrote:
>> > Currently everybody assumes (correctly) that the element is completely
>> > removed (Ajax and non-Ajax), i.e. not present in the final markup.
>> > This means that scripts that iterate through the dom, or check for the
>> > document.getElementById() == null will fail if we implement this.
>> then you'll have to check for document.getElementById().style.display ==
>> "none"
>> it's a bit longer to write, but it is semantically better. Indeed it
>> checks a component *visibility*, and not its existence.
>>
>> When I do setVisible(false), I expect setVisible(true) to work if called
>> later, in ajax or not. Currently, it doesn't work in Ajax. Don't you
>> think it is a real bug?
>> If yes, I don't think breaking such scripts should be used as a pretext
>> not to fix bugs ;-)
>>
>>
>> Fred
>>
>
>


-- 
Frédéric BERTIN
Contrôle industriel
ANYWARE TECHNOLOGIES
Tél (direct) : +33 (0)5.61.00.06.41
Tél (standard) : +33 (0)5.61.00.52.90
Fax : +33 (0)5 61 00 51 46
http://www.anyware-tech.com


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> But why can't it be "per component" setting? Why can't we have a flag for this?

It can, but having to set it per component will require you to do it
everywhere. This might make the change less interesting, and less
consistent. If you make it an application setting, the whole
application willl work in the same way, ajax or non-ajax. This is then
something where designers and JavaScript builders can work with.

Martijn

>
> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > It is not a bug, but the current way how markup rendering works. It
> > has been this way since 0.1. It is widely published behavior and
> > widely used. Introducing this will create bugs in existing
> > applications, that are hard to track.
> >
> > And please, don't give me JavaScript lessons, thank you very much, I
> > find that very condescending. I understand you find this solution very
> > elegant and perfect for *YOUR* usecase. As a framework
> > builder/maintainer I have to weigh existing investments as well. I
> > don't take breaking existing applications lightly.
> >
> > *IF* we were to adopt this, it should not be the default (because it
> > breaks existing applications), and it should be an application setting
> > to turn it on, or a page setting (which inherits the application
> > setting?). I think that making it a WMC specific setting will mitigate
> > the advantages of this approach.
> >
> > As for the migration guides, these are usually properly ignored. If
> > you can fail fast, then we should do that. Typically we use changes in
> > API and @deprecation for that. This change doesn't have any of those
> > safeguards.
> >
> > Martijn
> >
> >
> > On 3/19/07, Frédéric Bertin <fr...@anyware-tech.com> wrote:
> > > Martijn Dashorst wrote:
> > > > Currently everybody assumes (correctly) that the element is completely
> > > > removed (Ajax and non-Ajax), i.e. not present in the final markup.
> > > > This means that scripts that iterate through the dom, or check for the
> > > > document.getElementById() == null will fail if we implement this.
> > > then you'll have to check for document.getElementById().style.display ==
> > > "none"
> > > it's a bit longer to write, but it is semantically better. Indeed it
> > > checks a component *visibility*, and not its existence.
> > >
> > > When I do setVisible(false), I expect setVisible(true) to work if called
> > > later, in ajax or not. Currently, it doesn't work in Ajax. Don't you
> > > think it is a real bug?
> > > If yes, I don't think breaking such scripts should be used as a pretext
> > > not to fix bugs ;-)
> > >
> > >
> > > Fred
> > >
> >
> >
> > --
> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > Join the wicket community at irc.freenode.net: ##wicket
> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > http://wicketframework.org
> >
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
But why can't it be "per component" setting? Why can't we have a flag for this?

On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> It is not a bug, but the current way how markup rendering works. It
> has been this way since 0.1. It is widely published behavior and
> widely used. Introducing this will create bugs in existing
> applications, that are hard to track.
>
> And please, don't give me JavaScript lessons, thank you very much, I
> find that very condescending. I understand you find this solution very
> elegant and perfect for *YOUR* usecase. As a framework
> builder/maintainer I have to weigh existing investments as well. I
> don't take breaking existing applications lightly.
>
> *IF* we were to adopt this, it should not be the default (because it
> breaks existing applications), and it should be an application setting
> to turn it on, or a page setting (which inherits the application
> setting?). I think that making it a WMC specific setting will mitigate
> the advantages of this approach.
>
> As for the migration guides, these are usually properly ignored. If
> you can fail fast, then we should do that. Typically we use changes in
> API and @deprecation for that. This change doesn't have any of those
> safeguards.
>
> Martijn
>
>
> On 3/19/07, Frédéric Bertin <fr...@anyware-tech.com> wrote:
> > Martijn Dashorst wrote:
> > > Currently everybody assumes (correctly) that the element is completely
> > > removed (Ajax and non-Ajax), i.e. not present in the final markup.
> > > This means that scripts that iterate through the dom, or check for the
> > > document.getElementById() == null will fail if we implement this.
> > then you'll have to check for document.getElementById().style.display ==
> > "none"
> > it's a bit longer to write, but it is semantically better. Indeed it
> > checks a component *visibility*, and not its existence.
> >
> > When I do setVisible(false), I expect setVisible(true) to work if called
> > later, in ajax or not. Currently, it doesn't work in Ajax. Don't you
> > think it is a real bug?
> > If yes, I don't think breaking such scripts should be used as a pretext
> > not to fix bugs ;-)
> >
> >
> > Fred
> >
>
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
It is not a bug, but the current way how markup rendering works. It
has been this way since 0.1. It is widely published behavior and
widely used. Introducing this will create bugs in existing
applications, that are hard to track.

And please, don't give me JavaScript lessons, thank you very much, I
find that very condescending. I understand you find this solution very
elegant and perfect for *YOUR* usecase. As a framework
builder/maintainer I have to weigh existing investments as well. I
don't take breaking existing applications lightly.

*IF* we were to adopt this, it should not be the default (because it
breaks existing applications), and it should be an application setting
to turn it on, or a page setting (which inherits the application
setting?). I think that making it a WMC specific setting will mitigate
the advantages of this approach.

As for the migration guides, these are usually properly ignored. If
you can fail fast, then we should do that. Typically we use changes in
API and @deprecation for that. This change doesn't have any of those
safeguards.

Martijn


On 3/19/07, Frédéric Bertin <fr...@anyware-tech.com> wrote:
> Martijn Dashorst wrote:
> > Currently everybody assumes (correctly) that the element is completely
> > removed (Ajax and non-Ajax), i.e. not present in the final markup.
> > This means that scripts that iterate through the dom, or check for the
> > document.getElementById() == null will fail if we implement this.
> then you'll have to check for document.getElementById().style.display ==
> "none"
> it's a bit longer to write, but it is semantically better. Indeed it
> checks a component *visibility*, and not its existence.
>
> When I do setVisible(false), I expect setVisible(true) to work if called
> later, in ajax or not. Currently, it doesn't work in Ajax. Don't you
> think it is a real bug?
> If yes, I don't think breaking such scripts should be used as a pretext
> not to fix bugs ;-)
>
>
> Fred
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Frédéric Bertin <fr...@anyware-tech.com>.
Martijn Dashorst wrote:
> Currently everybody assumes (correctly) that the element is completely
> removed (Ajax and non-Ajax), i.e. not present in the final markup.
> This means that scripts that iterate through the dom, or check for the
> document.getElementById() == null will fail if we implement this.
then you'll have to check for document.getElementById().style.display == 
"none"
it's a bit longer to write, but it is semantically better. Indeed it 
checks a component *visibility*, and not its existence.

When I do setVisible(false), I expect setVisible(true) to work if called 
later, in ajax or not. Currently, it doesn't work in Ajax. Don't you 
think it is a real bug?
If yes, I don't think breaking such scripts should be used as a pretext 
not to fix bugs ;-)


Fred

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
Currently everybody assumes (correctly) that the element is completely
removed (Ajax and non-Ajax), i.e. not present in the final markup.
This means that scripts that iterate through the dom, or check for the
document.getElementById() == null will fail if we implement this.

I *strongly* discourage changing this behavior.

Martijn

On 3/19/07, Matej Knopp <ma...@gmail.com> wrote:
> Will it? This seems to be actually quite a smart workaround. How
> exactly will this break existing clients? Only thing i'm concerned
> about is the validity of output markup. but imho when we preserve the
> original tag names, e.g. td will render as td, it should be all right.
>
> -Matej
>
> On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > So you mean:
> >
> >     Label l = Label("foo", "hello");
> > renders:
> >     <span wicket:id="foo">hello</span>
> >
> > ... some ajax stuff, or a normal page render:
> >
> >     l.setVisible(false);
> > renders:
> >     <span wicket:id="foo" style="display:none"></span>
> > ?
> >
> > This can and will break existing clients in a very nasty manner,
> > because the markup id is still present in the final markup.
> >
> > Martijn
> >
> > On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > > Johan Compagner a écrit :
> > > >> > Also always just rendering the component but use the style to make in
> > > >> > invisible
> > > >> > could be a security problem. So that can't be the default.
> > > >>
> > > >> What do you mean by security problem?
> > > >
> > > >
> > > >
> > > > If the the component that is  set to none visible is none visible
> > > > because of
> > > > security
> > > > So it has data that never should be send to the browser because the
> > > > user is
> > > > not allowed
> > > > to see it.
> > >
> > > But data is never send to the user because a none visible component will
> > > be render as an empty tag, so without data
> > >
> > >
> >
> >
> > --
> > Learn Wicket at ApacheCon Europe: http://apachecon.com
> > Join the wicket community at irc.freenode.net: ##wicket
> > Wicket 1.2.5 will keep your server alive. Download Wicket now!
> > http://wicketframework.org
> >
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Matej Knopp <ma...@gmail.com>.
Will it? This seems to be actually quite a smart workaround. How
exactly will this break existing clients? Only thing i'm concerned
about is the validity of output markup. but imho when we preserve the
original tag names, e.g. td will render as td, it should be all right.

-Matej

On 3/19/07, Martijn Dashorst <ma...@gmail.com> wrote:
> So you mean:
>
>     Label l = Label("foo", "hello");
> renders:
>     <span wicket:id="foo">hello</span>
>
> ... some ajax stuff, or a normal page render:
>
>     l.setVisible(false);
> renders:
>     <span wicket:id="foo" style="display:none"></span>
> ?
>
> This can and will break existing clients in a very nasty manner,
> because the markup id is still present in the final markup.
>
> Martijn
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> > Johan Compagner a écrit :
> > >> > Also always just rendering the component but use the style to make in
> > >> > invisible
> > >> > could be a security problem. So that can't be the default.
> > >>
> > >> What do you mean by security problem?
> > >
> > >
> > >
> > > If the the component that is  set to none visible is none visible
> > > because of
> > > security
> > > So it has data that never should be send to the browser because the
> > > user is
> > > not allowed
> > > to see it.
> >
> > But data is never send to the user because a none visible component will
> > be render as an empty tag, so without data
> >
> >
>
>
> --
> Learn Wicket at ApacheCon Europe: http://apachecon.com
> Join the wicket community at irc.freenode.net: ##wicket
> Wicket 1.2.5 will keep your server alive. Download Wicket now!
> http://wicketframework.org
>

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Martijn Dashorst <ma...@gmail.com>.
So you mean:

    Label l = Label("foo", "hello");
renders:
    <span wicket:id="foo">hello</span>

... some ajax stuff, or a normal page render:

    l.setVisible(false);
renders:
    <span wicket:id="foo" style="display:none"></span>
?

This can and will break existing clients in a very nasty manner,
because the markup id is still present in the final markup.

Martijn

On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
> Johan Compagner a écrit :
> >> > Also always just rendering the component but use the style to make in
> >> > invisible
> >> > could be a security problem. So that can't be the default.
> >>
> >> What do you mean by security problem?
> >
> >
> >
> > If the the component that is  set to none visible is none visible
> > because of
> > security
> > So it has data that never should be send to the browser because the
> > user is
> > not allowed
> > to see it.
>
> But data is never send to the user because a none visible component will
> be render as an empty tag, so without data
>
>


-- 
Learn Wicket at ApacheCon Europe: http://apachecon.com
Join the wicket community at irc.freenode.net: ##wicket
Wicket 1.2.5 will keep your server alive. Download Wicket now!
http://wicketframework.org

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Johan Compagner a écrit :
>> > Also always just rendering the component but use the style to make in
>> > invisible
>> > could be a security problem. So that can't be the default.
>>
>> What do you mean by security problem?
>
>
>
> If the the component that is  set to none visible is none visible 
> because of
> security
> So it has data that never should be send to the browser because the 
> user is
> not allowed
> to see it.

But data is never send to the user because a none visible component will 
be render as an empty tag, so without data


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Johan Compagner <jc...@gmail.com>.
> > Also always just rendering the component but use the style to make in
> > invisible
> > could be a security problem. So that can't be the default.
>
> What do you mean by security problem?



If the the component that is  set to none visible is none visible because of
security
So it has data that never should be send to the browser because the user is
not allowed
to see it.

Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Vincent Demay <vi...@anyware-tech.com>.
Johan Compagner a écrit :
> But your patch doesn't do much with style i believe?
> It only generates a span tag where the component would be?
Sorry I made a mistake in the attachement : Here is the right one
https://issues.apache.org/jira/secure/attachment/12353648/component-real.txt 

> Is that allowed every where?
No it isn't allow in table for exemple
>
> Also always just rendering the component but use the style to make in
> invisible
> could be a security problem. So that can't be the default.

What do you mean by security problem?
>
> But i do like to have some option where i can say render that span (if 
> that
> is always possible) or use style
>
> johan
>
>
> On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>>
>> Hi,
>>     In the current implementation, going from setVisible(false) to
>> setVisible(true) in ajax does not work, The current workaround of this
>> bug is to surround the element with an other and to rerender the
>> surounding element. I found this way not very smart and too frustating
>> for the user. We actually need to surounding because nothing is render
>> when setVisible is false.
>>
>>     In https://issues.apache.org/jira/browse/WICKET-365 issue, I propose
>> a fix. This fix will simply render a tag with an id and a style
>> display:none. I don't think it is very expensive and works well with all
>> browsers (IE, FF, Safari) and also in html table (often a problem in
>> IE). With this patch it is possible to change the visibility of a
>> component in a page without surounding it and adding it to the target.
>>     This issue has been tagged as won't fix, But I still think it is a
>> good stuff
>>
>>     For exemple  a template(1) with foo.setVisible(false) and
>> bar.setVisible(false) will be render as (2). See
>>
>> https://issues.apache.org/jira/secure/attachment/12353204/Component.patch.txt 
>>
>>
>> (1)
>> <div wicket:id="bar">
>>     <div>Inner</div>
>>     <p>lsdkqjlqkdsfj</p>
>>     ... a lot of inner content
>> </div>
>> <table>
>>      <tr>
>>        <td>
>>              ....
>>        </td>
>>     </tr>
>>     <tr wicket:id="foo">
>>        <td>bla bla</td>
>>     </tr>
>> </table>
>>
>>
>>
>> (2)
>> <div id="bar0" style="display:none"></div>
>> <table>
>>      <tr>
>>        <td>
>>              ....
>>        </td>
>>     </tr>
>>     <tr id="foo1" style="display:none"></tr>
>> </table>
>>
>> I think this patch can make setVisible(false -> true) in ajax smarter
>> and more intuitive for user.
>>
>> WDYT?
>>
>>
>>
>>
>


Re: Going from setVisible(false) to setVisible(true) does not work without surounding element

Posted by Johan Compagner <jc...@gmail.com>.
But your patch doesn't do much with style i believe?
It only generates a span tag where the component would be?
Is that allowed every where?

Also always just rendering the component but use the style to make in
invisible
could be a security problem. So that can't be the default.

But i do like to have some option where i can say render that span (if that
is always possible) or use style

johan


On 3/19/07, Vincent Demay <vi...@anyware-tech.com> wrote:
>
> Hi,
>     In the current implementation, going from setVisible(false) to
> setVisible(true) in ajax does not work, The current workaround of this
> bug is to surround the element with an other and to rerender the
> surounding element. I found this way not very smart and too frustating
> for the user. We actually need to surounding because nothing is render
> when setVisible is false.
>
>     In https://issues.apache.org/jira/browse/WICKET-365 issue, I propose
> a fix. This fix will simply render a tag with an id and a style
> display:none. I don't think it is very expensive and works well with all
> browsers (IE, FF, Safari) and also in html table (often a problem in
> IE). With this patch it is possible to change the visibility of a
> component in a page without surounding it and adding it to the target.
>     This issue has been tagged as won't fix, But I still think it is a
> good stuff
>
>     For exemple  a template(1) with foo.setVisible(false) and
> bar.setVisible(false) will be render as (2). See
>
> https://issues.apache.org/jira/secure/attachment/12353204/Component.patch.txt
>
> (1)
> <div wicket:id="bar">
>     <div>Inner</div>
>     <p>lsdkqjlqkdsfj</p>
>     ... a lot of inner content
> </div>
> <table>
>      <tr>
>        <td>
>              ....
>        </td>
>     </tr>
>     <tr wicket:id="foo">
>        <td>bla bla</td>
>     </tr>
> </table>
>
>
>
> (2)
> <div id="bar0" style="display:none"></div>
> <table>
>      <tr>
>        <td>
>              ....
>        </td>
>     </tr>
>     <tr id="foo1" style="display:none"></tr>
> </table>
>
> I think this patch can make setVisible(false -> true) in ajax smarter
> and more intuitive for user.
>
> WDYT?
>
>
>
>