Niall Pemberton wrote:
I was thinking about this some more. Both you and the bugzilla ticket had
this issue
where theres an either/or situation with attributes - (e.g. either alt or
altKey) - maybe
we should stick focused on that. Attributes that are just "passed through"
your tag
files shouldn't be an issue? That way, I don't think we have to worry too
much about
compatibility. First thing to do is identify the tags/attributes that need
changing -
probably best to post that on the bugzilla ticket. Then we can discuss what
to do.

I think this is an issue *anywhere* a tag behaves differently on an attribute set to "" vs. an attribute set to null, since in this use case every attribute (or at least every one the tag file cares about passing on) will get set... :-(

My comment was "not that interested in working on the other taglibs" - even
if someone
else puts in a patch, I find that what takes the most time is testing -
writing the code
is usually pretty quick. Anyway, if you want to scope out what you think
needs changing
then we can take it from there.

Sure; maybe some of the testing burden can be mitigated through unit tesst (not sure if Struts has the infrastructure in place to unit test custom tags?)

L.


Niall

----- Original Message ----- From: "Laurie Harper" <[EMAIL PROTECTED]>
To: <user@struts.apache.org>


Cross-posting to the dev list for committer approval of the proposed
changes:

Niall Pemberton wrote:

I agree, not trimming!

Personally I don't have a problem with making the change (i.e. checking

for

zero length strings) in the html taglib, since an easy one line change

will

catch alot of them. I'm not that interested in doing it for the other
taglibs though.

Not that interested in doing the work or not that keen on seeing the
changes made? ;-) If the former, I'll be happy to put together patches
for each taglib to keep things consistent.


The way to do it would be to attach a patch to the bug and then ask for
objections before committing it. Besides changing BaseHandlerTag, do you
have any idea of how many other tags might need custom changes. The ones
that spring to mindare the ones that take action/forward/page/href etc.

as

attributes to generate urls

Well, I'm not a committer, but I get the point ;-) I haven't looked at
how many other places may need to change, though in most cases a single
change in a helper method should cover a bunch of them at once (e.g. in
TagUtils.computeURLWithCharEncoding() for the ones you mention above).

I'll take a more detailed look if/when such a change is approved in
principle by the committers.

L.


Niall

----- Original Message ----- From: "Laurie Harper" <[EMAIL PROTECTED]>
Sent: Wednesday, September 14, 2005 2:13 AM




Thanks Niall, yes, that's exactly what I'm talking about (thanks for the
link).

OK, so the issue is the impact on backwards compatibility if the tags
change such that empty values suppress attribute rendering. One
possibility would be to test for null or empty string, without trimming.
That would support the case you mentioned below where someone was
relying on rendering value=" ". It's still a behavioural change, though.

My vote would definately be to make the change suggested in 33064, but I
can see why that might be unacceptable. Short of adding a new attribute
to all tags to switch this behaviour on, or a global config option, I
don't see any way to resolve the issue that does retain backwards
compatibility though :-(

I suppose I should take this up on the dev list.

L.

Niall Pemberton wrote:


Laurie

Theres an open bugzilla ticket for the same kind of thing:

 http://issues.apache.org/bugzilla/show_bug.cgi?id=33064

Changing the prepareAttribute() method in o.a.s.t.h.BaseHandlerTag will

deal


with alot of the html taglib attributes that are simply output. For
alt/altkey and title/titleKey - the message() method in BaseHandlerTag

deals


with those attributes.

Having said that I added a trim() to the label for the SubmitTag a

while

ago


and that caused a problem for someone who relied on the fact that it

would


emit value=" " so I'm wondering if adding a length() check is going to

cause


anyone else issues?


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to