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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]>
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 <[EMAIL PROTECTED]>
> > 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
> > > >>> >> >
> > > >>> >>
> > > >>> >
> > > >>> >
> > > >>>
> > > >>>
> > > >>
> > > >>
> > > >
> > > >
> > >
> > >
> >
>
