Gang -
We have added the following two methods to TagAttribute (and similar
methods to TagAttributes):
Index: javax/faces/view/facelets/TagAttribute.java
===================================================================
--- javax/faces/view/facelets/TagAttribute.java (revision 8845)
+++ javax/faces/view/facelets/TagAttribute.java (revision 11719)
@@ -64,8 +64,8 @@
+ /**
+ * <p class="changed_added_2_2">A reference to the Tag for which
this class
+ * represents the attributes. For compatibility with previous
implementations,
+ * an implementation is provided that returns {_at_code null}.</p>
+ *
+ * @since 2.2
+ *
+ * @return the {_at_link Tag} for which this class represents the
attributes.
+ */
+
+ public Tag getTag() {
+ return null;
+ }
-
+ /**
+ * <p class="changed_added_2_2">Set a reference to the Tag for which
+ * this class represents the attributes. The VDL runtime must
+ * ensure that this method is called before any {_at_link
+ * FaceletHandler}s for this element are instantiated. For
+ * compatibility with previous implementations, a no-op
+ * implementation is provided.</p>
+ *
+ * @since 2.2
+ *
+ */
+ public void setTag(Tag tag) {
+
+ }
Prior to the addition of setTag(), TagAttribute exposes a
read-only/immutable API. Introducing mutability into this contract
seems like an undesirable change.
I was wondering whether we can find some way to avoid this, eg. only
expose the getter in the public API and instead pass the Tag into the
impl class through some other means.
However, while doing so I noticed that these methods don't seem to be
needed at all
The one place where Mojarra calls TagAttribute.getTag() is in the
(internal) DefaultTagDecorator:
> protected TagAttribute convertTagAttribute(TagAttribute
attribute) {
> Location location = attribute.getLocation();
> String ns = attribute.getNamespace();
> String myLocalName = attribute.getLocalName();
> String qName;
> String value = attribute.getValue();
>
> if (Namespace.jsf.uri.equals(attribute.getNamespace())) {
> // make this a component
attribute
> qName = myLocalName;
> ns = "";
> } else {
> if (ns.length() != 0 &&
!ns.equals(attribute.getTag().getNamespace(\ <--------- Here!
> ))) {
> // the attribute has a different namespace than
the tag. preser\
> ve
it.
> return attribute;
> }
The problem with this is: we've already got access to the Tag instance.
convertTagAttribute() is called from convertAttributes(). This is
called from convertTag(), which gets passed the Tag. Since this is all
internal code in a package-private class, we can easily solve this by
passing the Tag instance into convertTagAttributes().
Why are we adding new public APIs rather than just refactoring this
code? Unless I am missing some other use case, let's kill the new APIs
and do the trivial implementation refactoring. (Same thing applies to
TagAttributes.)
BTW, although we should kill these new methods, they provide a good
example of why adding "non-transparent" default implementations (such as
returning null) to abstract classes are a bad idea.
TagAttribute.getTag() javadoc states that:
+ * represents the attributes. For compatibility with previous
implementations,
+ * an implementation is provided that returns {_at_code null}.</p>
I suppose the idea is that, for those random folks who have implemented
TagAttribute subclasses, they are now happy because they are not exposed
to this new API requirement.
However, take a look at the getTag() usage above. Is this default
implementation going to help? Not really. It's simply going to lead to
a much more difficult to diagnose NullPointerException.
As someone who has extended just about every extension point that JSF
offers (though not TagAttribute), I would much, much rather find out
about this requirement at compile time. Providing a default
implementation that returns null (and later NPEs!) provides an false
sense of compatibility, but really just ends up hiding the requirement
and wasting developer time.
Andy