You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Garret Wilson <ga...@globalmentor.com> on 2014/07/12 22:53:55 UTC

avoiding hard-coding HTML names

Hi, all. I notice that throughout the code HTML element and attribute 
names are hard-coded. Does Wicket have a list of HTML name and attribute 
constants that we could reference? I would find that a lot safer, and 
perhaps even more readable. Not only that, it would allow us to quickly 
find all the places that use the e.g. HTML.Element.EM element.

Secondly, in the same vein, I see lots of code like this:

if(tag.getName().equalsIgnoreCase("a"))...

That's also tedious and error-prone---I can't trust myself not to forget 
to do a case-insensitive comparison. Perhaps there could be a 
ComponentTag.hasName() method that does this for me? So the above would be:

if(tag.hasName(HTML.Element.EM))...

On the other hand, perhaps Wicket is wanting to support other contexts 
such as general XML, in which case case-sensitive matching is necessary. 
(I would imagine that the entire codebase plays loosely with whether or 
not matching is case-sensitive, though. More research is necessary on my 
part.)

What do you think at least about having HTML element/attribute 
constants? That my be an area I can contribute to in the short term.

Garret

Re: avoiding hard-coding HTML names

Posted by Martin Grigorov <mg...@apache.org>.
Hi,

Feel free to create a Pull Request (at GitHub) or attach a patch (though it
is harder to review & comment when working with patches) and we can discuss
this further.
It is just that I don't see much value in this suggestion. There were no
problems with the hardcoded tag names in some of the components for the
last 10 years. I don't see why these problems will start now.

I see some value in having constants for the JS event names. Since
6.0 Wicket uses event binding so the event name should be "click" instead
of "onclick" (as in Wicket 1.5 and earlier), so AjaxEventBehavior and
BaseWicketTester have logic to cut the leading "on" if it is there. Even
with constants we will have to check again but maybe this way the
developers will migrate from "onevent" to "event" sooner.
Adding a LOG.warn() to
https://github.com/apache/wicket/blob/442932d4e4c5cc27940bc2ef956cb24c1ba54df0/wicket-core/src/main/java/org/apache/wicket/ajax/AjaxEventBehavior.java#L70
will help better.

On Sat, Jul 19, 2014 at 1:43 AM, Garret Wilson <ga...@globalmentor.com>
wrote:

> On 7/18/2014 5:14 PM, Martin Grigorov wrote:
>
>> On Jul 18, 2014 8:10 PM, "Garret Wilson" <ga...@globalmentor.com> wrote:
>>
>>> I wasn't trying to "validate" or "restrict" HTML tag names---I just
>>>
>> thought it helpful to provide some constants for the HTML tags we use all
>> the time, so that some programmer (e.g. me) doesn't make a typo.
>>
>> do you suggest to add constants only for the few HTML tags used in Wicket
>> (like 'a', 'button', 'link') ?
>>
>
> Nope, I was suggesting adding constants for all HTML5 elements. The HTML5
> recommendation is a specification---they are quite specific (pun
> accidental) about what the elements are. I'll happily volunteer to create
> constants for all of them.
>
> (Actually, do you know what would be really cool? I'm all for
> modularization---it would me nice to have a tiny separate HTML5 Definitions
> library Wicket could pull in,

and that others in software-land could use as well. It would contain all
> the HTML5 elements and attributes.)
>
> In no way am I wanting to restrict developers from putting out a "foo"
> tag. I'm just a little surprised that there would be an /aversion/ to using
> constants.
>
> You know, since I'm new to this l list, I thought that before making
> suggestions about big, complicated new features that would be
> controversial, I thought I'd start by making some obvious suggestions that
> no one could ever have a problem with, and which hadn't got done simply
> because there weren't enough hands to do the work. But I'm running into
> controversy even regarding using constants instead of hard-coded values! I
> guess I'm a little taken aback.
>
>
>  ...
>>
>>
>>  Do you see any value in adding a boolean Tag.hasName(String) method that
>>>
>> handles the equalsIgnoreCase()? I don't even trust myself to
>>
>> how to make sure that XML producing pages won't use this method ?
>>
>
> How do you know which pages are XML producing pages and which aren't? How
> do you know which Wicket component is going to have a hard-coded
> tagName.equals() and a hard-coded tagName.equalsIgnoreCase()? I think your
> question is indicative of a wider problem of Wicket playing loosely with
> whether it's working with HTML or XHTML. Perhaps we need to have that
> discussion first.
>
> But encapsulating the comparison semantics into a separate method is a
> step towards addressing the larger issue, because rather than hard-coding
> the comparison logic into the page itself, you delegate to the tag. The day
> in which Wicket addresses the ambiguity between whether it is working with
> HTML or XHTML, then the Tag instance you get would do the correct
> comparison automatically (e.g. HTMLTag.hasName() or XMLTag.hasName()) .
>
> So using constant definitions for tag names, along with semantically
> significant methods for delegating logic, are in my opinion two approaches
> to addressing the HTML/XHTML problem, not making it worse.
>
> But I don't want to seem argumentative. I'll drop this and go try to find
> yet another simple Wicket task that won't prove so contentious.


>
> Garret
>

Re: avoiding hard-coding HTML names

Posted by Garret Wilson <ga...@globalmentor.com>.
On 7/18/2014 5:14 PM, Martin Grigorov wrote:
> On Jul 18, 2014 8:10 PM, "Garret Wilson" <ga...@globalmentor.com> wrote:
>> I wasn't trying to "validate" or "restrict" HTML tag names---I just
> thought it helpful to provide some constants for the HTML tags we use all
> the time, so that some programmer (e.g. me) doesn't make a typo.
>
> do you suggest to add constants only for the few HTML tags used in Wicket
> (like 'a', 'button', 'link') ?

Nope, I was suggesting adding constants for all HTML5 elements. The 
HTML5 recommendation is a specification---they are quite specific (pun 
accidental) about what the elements are. I'll happily volunteer to 
create constants for all of them.

(Actually, do you know what would be really cool? I'm all for 
modularization---it would me nice to have a tiny separate HTML5 
Definitions library Wicket could pull in, and that others in 
software-land could use as well. It would contain all the HTML5 elements 
and attributes.)

In no way am I wanting to restrict developers from putting out a "foo" 
tag. I'm just a little surprised that there would be an /aversion/ to 
using constants.

You know, since I'm new to this l list, I thought that before making 
suggestions about big, complicated new features that would be 
controversial, I thought I'd start by making some obvious suggestions 
that no one could ever have a problem with, and which hadn't got done 
simply because there weren't enough hands to do the work. But I'm 
running into controversy even regarding using constants instead of 
hard-coded values! I guess I'm a little taken aback.


> ...
>
>> Do you see any value in adding a boolean Tag.hasName(String) method that
> handles the equalsIgnoreCase()? I don't even trust myself to
>
> how to make sure that XML producing pages won't use this method ?

How do you know which pages are XML producing pages and which aren't? 
How do you know which Wicket component is going to have a hard-coded 
tagName.equals() and a hard-coded tagName.equalsIgnoreCase()? I think 
your question is indicative of a wider problem of Wicket playing loosely 
with whether it's working with HTML or XHTML. Perhaps we need to have 
that discussion first.

But encapsulating the comparison semantics into a separate method is a 
step towards addressing the larger issue, because rather than 
hard-coding the comparison logic into the page itself, you delegate to 
the tag. The day in which Wicket addresses the ambiguity between whether 
it is working with HTML or XHTML, then the Tag instance you get would do 
the correct comparison automatically (e.g. HTMLTag.hasName() or 
XMLTag.hasName()) .

So using constant definitions for tag names, along with semantically 
significant methods for delegating logic, are in my opinion two 
approaches to addressing the HTML/XHTML problem, not making it worse.

But I don't want to seem argumentative. I'll drop this and go try to 
find yet another simple Wicket task that won't prove so contentious.

Garret

Re: avoiding hard-coding HTML names

Posted by Martin Grigorov <mg...@apache.org>.
On Jul 18, 2014 8:10 PM, "Garret Wilson" <ga...@globalmentor.com> wrote:
>
> I wasn't trying to "validate" or "restrict" HTML tag names---I just
thought it helpful to provide some constants for the HTML tags we use all
the time, so that some programmer (e.g. me) doesn't make a typo.

do you suggest to add constants only for the few HTML tags used in Wicket
(like 'a', 'button', 'link') ?
this will lead to a wave of questions on the lists why 'fieldset' is not
there too, for example
my point is that we don't want to maintain a huge list of tag names which
are not even used by Wicket itself

and what about "<em>" ?
I'd hate to have to write/read something like: "<"+HTMLElement.EM+">"

>
> Do you see any value in adding a boolean Tag.hasName(String) method that
handles the equalsIgnoreCase()? I don't even trust myself to

how to make sure that XML producing pages won't use this method ?

always remember to do the correct equals() on the tags. I'm an advocate for
helper methods that clarify semantics of what's being done, and also
protect the programmer from making mistakes.
>
> I hope you don't mind my making these suggestions, based upon my decades
of experience of trying to protect my code from me. ;)
>
> Garret
>
>
> On 7/17/2014 7:48 AM, Martijn Dashorst wrote:
>>
>> Not a Java or Wicket article, but interesting nonetheless:
>> http://inessential.com/2014/07/14/string_constants
>>
>> I'm not a big fan of putting everything in constants in a central
>> location. We won't be changing an anchor tag from a to anchor across
>> all code in Wicket, ever.
>>
>> There has to be a benefit to making a constant (string). For example,
>> what is more clear:
>>
>> public class SomeAbstractLink {
>>      protected void checkTag(Tag t) {
>>          assert t.getName().equals(HtmlTags.ANCHOR);
>>      }
>> }
>>
>> or
>>
>> public class SomeAbstractLink {
>>      protected void checkTag(Tag t) {
>>          assert t.getName().equalsIgnoreCase("a");
>>      }
>> }
>>
>> or possibly:
>>
>> public class SomeAbstractLink {
>>      protected void checkTag(Tag t) {
>>          TagAsserts.tagNameValid(t, "a");
>>      }
>> }
>>
>> Martijn
>>
>>
>> On Sat, Jul 12, 2014 at 10:53 PM, Garret Wilson <ga...@globalmentor.com>
wrote:
>>>
>>> Hi, all. I notice that throughout the code HTML element and attribute
names
>>> are hard-coded. Does Wicket have a list of HTML name and attribute
constants
>>> that we could reference? I would find that a lot safer, and perhaps even
>>> more readable. Not only that, it would allow us to quickly find all the
>>> places that use the e.g. HTML.Element.EM element.
>>>
>>> Secondly, in the same vein, I see lots of code like this:
>>>
>>> if(tag.getName().equalsIgnoreCase("a"))...
>>>
>>> That's also tedious and error-prone---I can't trust myself not to
forget to
>>> do a case-insensitive comparison. Perhaps there could be a
>>> ComponentTag.hasName() method that does this for me? So the above would
be:
>>>
>>> if(tag.hasName(HTML.Element.EM))...
>>>
>>> On the other hand, perhaps Wicket is wanting to support other contexts
such
>>> as general XML, in which case case-sensitive matching is necessary. (I
would
>>> imagine that the entire codebase plays loosely with whether or not
matching
>>> is case-sensitive, though. More research is necessary on my part.)
>>>
>>> What do you think at least about having HTML element/attribute
constants?
>>> That my be an area I can contribute to in the short term.
>>>
>>> Garret
>>
>>
>>
>

Re: avoiding hard-coding HTML names

Posted by Garret Wilson <ga...@globalmentor.com>.
I wasn't trying to "validate" or "restrict" HTML tag names---I just 
thought it helpful to provide some constants for the HTML tags we use 
all the time, so that some programmer (e.g. me) doesn't make a typo.

Do you see any value in adding a boolean Tag.hasName(String) method that 
handles the equalsIgnoreCase()? I don't even trust myself to always 
remember to do the correct equals() on the tags. I'm an advocate for 
helper methods that clarify semantics of what's being done, and also 
protect the programmer from making mistakes.

I hope you don't mind my making these suggestions, based upon my decades 
of experience of trying to protect my code from me. ;)

Garret

On 7/17/2014 7:48 AM, Martijn Dashorst wrote:
> Not a Java or Wicket article, but interesting nonetheless:
> http://inessential.com/2014/07/14/string_constants
>
> I'm not a big fan of putting everything in constants in a central
> location. We won't be changing an anchor tag from a to anchor across
> all code in Wicket, ever.
>
> There has to be a benefit to making a constant (string). For example,
> what is more clear:
>
> public class SomeAbstractLink {
>      protected void checkTag(Tag t) {
>          assert t.getName().equals(HtmlTags.ANCHOR);
>      }
> }
>
> or
>
> public class SomeAbstractLink {
>      protected void checkTag(Tag t) {
>          assert t.getName().equalsIgnoreCase("a");
>      }
> }
>
> or possibly:
>
> public class SomeAbstractLink {
>      protected void checkTag(Tag t) {
>          TagAsserts.tagNameValid(t, "a");
>      }
> }
>
> Martijn
>
>
> On Sat, Jul 12, 2014 at 10:53 PM, Garret Wilson <ga...@globalmentor.com> wrote:
>> Hi, all. I notice that throughout the code HTML element and attribute names
>> are hard-coded. Does Wicket have a list of HTML name and attribute constants
>> that we could reference? I would find that a lot safer, and perhaps even
>> more readable. Not only that, it would allow us to quickly find all the
>> places that use the e.g. HTML.Element.EM element.
>>
>> Secondly, in the same vein, I see lots of code like this:
>>
>> if(tag.getName().equalsIgnoreCase("a"))...
>>
>> That's also tedious and error-prone---I can't trust myself not to forget to
>> do a case-insensitive comparison. Perhaps there could be a
>> ComponentTag.hasName() method that does this for me? So the above would be:
>>
>> if(tag.hasName(HTML.Element.EM))...
>>
>> On the other hand, perhaps Wicket is wanting to support other contexts such
>> as general XML, in which case case-sensitive matching is necessary. (I would
>> imagine that the entire codebase plays loosely with whether or not matching
>> is case-sensitive, though. More research is necessary on my part.)
>>
>> What do you think at least about having HTML element/attribute constants?
>> That my be an area I can contribute to in the short term.
>>
>> Garret
>
>


Re: avoiding hard-coding HTML names

Posted by Martijn Dashorst <ma...@gmail.com>.
Not a Java or Wicket article, but interesting nonetheless:
http://inessential.com/2014/07/14/string_constants

I'm not a big fan of putting everything in constants in a central
location. We won't be changing an anchor tag from a to anchor across
all code in Wicket, ever.

There has to be a benefit to making a constant (string). For example,
what is more clear:

public class SomeAbstractLink {
    protected void checkTag(Tag t) {
        assert t.getName().equals(HtmlTags.ANCHOR);
    }
}

or

public class SomeAbstractLink {
    protected void checkTag(Tag t) {
        assert t.getName().equalsIgnoreCase("a");
    }
}

or possibly:

public class SomeAbstractLink {
    protected void checkTag(Tag t) {
        TagAsserts.tagNameValid(t, "a");
    }
}

Martijn


On Sat, Jul 12, 2014 at 10:53 PM, Garret Wilson <ga...@globalmentor.com> wrote:
> Hi, all. I notice that throughout the code HTML element and attribute names
> are hard-coded. Does Wicket have a list of HTML name and attribute constants
> that we could reference? I would find that a lot safer, and perhaps even
> more readable. Not only that, it would allow us to quickly find all the
> places that use the e.g. HTML.Element.EM element.
>
> Secondly, in the same vein, I see lots of code like this:
>
> if(tag.getName().equalsIgnoreCase("a"))...
>
> That's also tedious and error-prone---I can't trust myself not to forget to
> do a case-insensitive comparison. Perhaps there could be a
> ComponentTag.hasName() method that does this for me? So the above would be:
>
> if(tag.hasName(HTML.Element.EM))...
>
> On the other hand, perhaps Wicket is wanting to support other contexts such
> as general XML, in which case case-sensitive matching is necessary. (I would
> imagine that the entire codebase plays loosely with whether or not matching
> is case-sensitive, though. More research is necessary on my part.)
>
> What do you think at least about having HTML element/attribute constants?
> That my be an area I can contribute to in the short term.
>
> Garret



-- 
Become a Wicket expert, learn from the best: http://wicketinaction.com

Re: avoiding hard-coding HTML names

Posted by Martijn Dashorst <ma...@gmail.com>.
On Thu, Jul 17, 2014 at 1:09 PM, Martin Grigorov <mg...@apache.org> wrote:
> The main problem I see with this is that we cannot cover all possible names.
> 1) It is possible to create custom HTML elements in JavaScript. So the list
> of names should be easy to extend

+1

> 2) with Web Components standard custom names are used even more often

+1

> 3) and yes, some users use Wicket to generate XML/SVG/... where case
> sensitivity is important

+1

> I remember some users wanted to have a list of JS events. The main problem
> with this before that we had to have both "click" and "onclick" before 6.0.
> Now we don't need the onXYZ version. But again it should be easy to extend
> because using custom JS event names is also very common in development.

Also a strict dependency on a particular standard is hard to maintain
if you want usable applications. For example we have a checkType in
form component that was a pain with the new markup elements.

> Are there that many occurrences of such hardcoded HTML element and event
> names ? I don't remember having any problems with them for the last 4 years.

AFIAK the only issues were what I mentioned above: to be too strict in
what we deem acceptable.

Martijn

Re: avoiding hard-coding HTML names

Posted by Martin Grigorov <mg...@apache.org>.
Hi,

The main problem I see with this is that we cannot cover all possible names.
1) It is possible to create custom HTML elements in JavaScript. So the list
of names should be easy to extend
2) with Web Components standard custom names are used even more often
3) and yes, some users use Wicket to generate XML/SVG/... where case
sensitivity is important

I remember some users wanted to have a list of JS events. The main problem
with this before that we had to have both "click" and "onclick" before 6.0.
Now we don't need the onXYZ version. But again it should be easy to extend
because using custom JS event names is also very common in development.

Are there that many occurrences of such hardcoded HTML element and event
names ? I don't remember having any problems with them for the last 4 years.

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov


On Sat, Jul 12, 2014 at 11:53 PM, Garret Wilson <ga...@globalmentor.com>
wrote:

> Hi, all. I notice that throughout the code HTML element and attribute
> names are hard-coded. Does Wicket have a list of HTML name and attribute
> constants that we could reference? I would find that a lot safer, and
> perhaps even more readable. Not only that, it would allow us to quickly
> find all the places that use the e.g. HTML.Element.EM element.
>
> Secondly, in the same vein, I see lots of code like this:
>
> if(tag.getName().equalsIgnoreCase("a"))...
>
> That's also tedious and error-prone---I can't trust myself not to forget
> to do a case-insensitive comparison. Perhaps there could be a
> ComponentTag.hasName() method that does this for me? So the above would be:
>
> if(tag.hasName(HTML.Element.EM))...
>
> On the other hand, perhaps Wicket is wanting to support other contexts
> such as general XML, in which case case-sensitive matching is necessary. (I
> would imagine that the entire codebase plays loosely with whether or not
> matching is case-sensitive, though. More research is necessary on my part.)
>
> What do you think at least about having HTML element/attribute constants?
> That my be an area I can contribute to in the short term.
>
> Garret
>