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 <[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
> > >>> >> >
> > >>> >>
> > >>> >
> > >>> >
> > >>>
> > >>>
> > >>
> > >>
> > >
> > >
> >
> >
>


Reply via email to