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

Vincent Demay Mon, 19 Mar 2007 08:07:35 -0800

Igor Vaynberg a écrit :

what you need is a different method. instead ofsetoutputmarkupid(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 currentbehavior 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, forthe 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 breakcss 2first child selector if that happens to be invisible because i dontthink itchecks for the display attribute. it will also break 3rd partyjavascript

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 isset 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 waythat 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. Silentfailures

> >> 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, orcheck for

> >>> the

> >>> > document.getElementById() == null will fail if we implementthis.

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

> >>> 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 Wicketnow!

> >>> >> > http://wicketframework.org
> >>> >> >
> >>> >>
> >>> >
> >>> >
> >>>
> >>>
> >>
> >>
> >
> >
>
>

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

Reply via email to