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