innerHTML for buttons

[Bob McElrath]

The functions createTiddlyButton and createTiddlyElement take an
argument theText which gets entered into the document with
document.createTextNode.

Is there any reason why this has to be a text node, rather than using
innerHTML? I switched them both to innerHTML and this doesn't seem to
cause any problem.

The reason I ask is that ZiddlyWiki comes with a theme (ZiddlyBlue)
created by Tim Morgan which uses images for some of the buttons. I
didn't like it at first but it's growing on me... Of course this can't
be done with createTextNode. (If you haven't seen this before you can
go to http://02.altair.zettai.net )

This interface definitely qualifies as "Mystery Meat Navigation":
http://en.wikipedia.org/wiki/Mystery_meat_navigation
but on the other hand TiddlyWiki is an intensely personal application,
and I for one don't need to read descriptions of what everything does in
an interface that I use every day. Brevity is powerful.

But that's neither here nor there...is there any reason not to use
innerHTML in these two places? Actually only createTiddlyButton is used
by this theme, but the idea applies to createTiddlyElement too...

Index: js/Dom.js
===================================================================
--- js/Dom.js (revision 643)
+++ js/Dom.js (working copy)
@@ -45,7 +45,7 @@
if(theID != null)
e.setAttribute("id",theID);
if(theText != null)
- e.appendChild(document.createTextNode(theText));
+ e.innerHTML = theText;
if(theParent != null)
theParent.appendChild(e);
return(e);
Index: js/Utilities.js
===================================================================
--- js/Utilities.js (revision 643)
+++ js/Utilities.js (working copy)
@@ -13,7 +13,7 @@
if(theTooltip)
theButton.setAttribute("title",theTooltip);
if(theText)
- theButton.appendChild(document.createTextNode(theText));
+ theButton.innerHTML = theText;
if(theClass)
theButton.className = theClass;
else

--
Cheers,
Bob McElrath [Univ. of California at Davis, Department of Physics]

Somewhere, something incredible is waiting to be known. - Carl Sagan

[Martin Budden]

I've just recently noticed this as well. I think the reason for using
document.createNode is historical, and using innerHTLM is a better
solution. I think you should raise a ticket for this.

Martin

[Jeremy Ruston]

Originally I tried to avoid innerHTML and do everything 'properly' via
DOM manipulation. Subsequently, like everyone else, I've discovered
just how painful is the speed penalty we pay for going the DOM route.

Bob's proposed change is not 100% compatible (intentionally, of
course) because it interprets the text parameter as HTML not plain
text, but I'd be happy to do accept it. Although I'd want to confirm
that there are no unpleasant side-effects on other browsers.

Cheers

Jeremy

--
Jeremy Ruston
mailto:jer...@osmosoft.com
http://www.tiddlywiki.com

[Bob McElrath]

Jeremy Ruston [jeremy...@gmail.com] wrote:
>
> Originally I tried to avoid innerHTML and do everything 'properly' via
> DOM manipulation. Subsequently, like everyone else, I've discovered
> just how painful is the speed penalty we pay for going the DOM route.

innerHTML is slower when modifying the content of a single node:
http://www.gtalbot.org/DHTMLSection/innerHTMLvsNodeValue.html
but vastly faster when creating complex content:
http://www.quirksmode.org/dom/innerhtml.html
which should come as no surprise. The DOM route involves tons of
javascript function calls, which incur a lot of overhead.

> Bob's proposed change is not 100% compatible (intentionally, of
> course) because it interprets the text parameter as HTML not plain
> text, but I'd be happy to do accept it. Although I'd want to confirm
> that there are no unpleasant side-effects on other browsers.

Revisions 668 and 669 contain the changes. We should check this
carefully. There are probably other places that need fixing. There are
many places that people might be depending on textNode behavior
implicitly, and others where the change makes sense. e.g.
config.macros.slider.createSlider -- with an image instead of a text
title -- why not?

I'd be open to suggestions for an alternate API where HTML is desired,
or simply a way to put images in places that now want text. Would
anyone want something more complicated than an image in
createTiddly(Button|Element)?

[Udo Borkowski]

I am fairly sure I have some code where I am passing a text to createTiddlyButton that is not valid HTML (e.g. containing < or > chars). Interpreting the text as HTML would break those buttons/elements.

What about introducing a convention: When the text passed in starts with "<html>" and ends with "</html>" the inner part will be taken as HTML.  Eg.

    <html>this is <b>bold</b> text</html>

vs.

    Submit this -->



Alternatively one could add a  parameter "isHTMLText" to createTiddlyButton/Element to avoid compatibility problems.


Udo

----------
Udo Borkowski
http://www.abego-software.de

[Clint Checketts]

Udo,

You have a good point that '<' or '>' might break it, but it seems like using those characters will be rare exceptions. I think it should be easier to enter html than wrapping it or setting a flag (ie on by default)

Could you use &gt; or &lt;?

I realize that TW should respect compatibility, but the angle brackets would be rare ducks.

-Clint

[Udo Borkowski]

Could you use &gt; or &lt;?

I could, but then the plugin would display a "&gt;" or "&lt;" in pre-2.1 TWs and "<" and ">" in TW 2.1 and later.

I.e. my example

Submit this -->

would be rendered a

    Submit this --&gt;


in TW 2.0

Udo

----------
Udo Borkowski
http://www.abego-software.de

[harryc]

I think Udo has a great solution. Defaulting to current behavior and
using a parameter to support the HTML option would allow the speedier
response for simple text objects or faster processing of the HTML (as
Bob points out) being passed with out breaking existing usage.

[Saq Imtiaz]

On 9/6/06, Udo Borkowski <Udo.Bo...@gmx.de> wrote:

I am fairly sure I have some code where I am passing a text to createTiddlyButton that is not valid HTML (e.g. containing < or > chars). Interpreting the text as HTML would break those buttons/elements.

What about introducing a convention: When the text passed in starts with "<html>" and ends with "</html>" the inner part will be taken as HTML.  Eg.

I second Udo's idea..... I think I have a few plugins as well that will break if the text passed is always treated as html. While I could always update the plugins, I still feel we should maintain backwards compatibility. We've taken so much trouble to do so for everything else, why stop now?

Cheers,
Saq

[Bob McElrath]

Saq Imtiaz [lew...@gmail.com] wrote:
> I second Udo's idea..... I think I have a few plugins as well that will
> break if the text passed is always treated as html. While I could always
> update the plugins, I still feel we should maintain backwards compatibility.
> We've taken so much trouble to do so for everything else, why stop now?

I don't like the isHTML flag. It just seems unnecessary. Should we
make other parameters to indicate what other parameters are? Parameters
are supposed to be indicated by their position. I would advocate a
second function createTiddlyHTMLButton...but I have another idea: allow
a DOM node to be passed as theText. That way the user can:

var e = createTiddlyElement("img");
e.src = "foo.gif";
createTiddlyButton(parent, e); // etc

...or use innerHTML if the user desires.

var e = document.createElement("span")
e.innerHTML = "<img src='foo.gif'/>";
createTiddlyButton(parent, e);

Thoughts?

Ok...on second thought I don't like that idea at all. The text gets
passed to these functions in roundabout ways, where it's not really
practical to place a DOM node. (For example, in this theme the image
comes in by way of config.commands.editTiddler.text which seems logical
to me)

So I seem to like the idea mentioned previously of allowing '<html>' to
delimit text. But barely. Since we don't wikify this text, it seems an
odd hackish solution. I prefer the innerHTML which was more
straightforward.

Ok I'm beginning to really hate svn. It seems there's no way to revert
the checked-in change I sent previously. I have to edit out all the
changes in -r668 and -r669, by hand! Then check in new changes that
undoes 668 and 669 and adds the below! AARRGH!!! Fortunately this is
a small change. I'd hate to have to undo a large checked-in change!

Idea #1: (the bad one)

Index: js/Dom.js
===================================================================
--- js/Dom.js (revision 667)
+++ js/Dom.js (working copy)
@@ -45,7 +45,10 @@

if(theID != null)
e.setAttribute("id",theID);
if(theText != null)
- e.appendChild(document.createTextNode(theText));

+ if(typeof theText == "object")
+ e.appendChild(theText);
+ else
+ e.appendChild(document.createTextNode(theText));

if(theParent != null)
theParent.appendChild(e);
return(e);
Index: js/Utilities.js
===================================================================

--- js/Utilities.js (revision 667)
+++ js/Utilities.js (working copy)
@@ -13,7 +13,10 @@

if(theTooltip)
theButton.setAttribute("title",theTooltip);
if(theText)
- theButton.appendChild(document.createTextNode(theText));

+ if(typeof theText == "object")
+ theButton.appendChild(theText);
+ else
+ theButton.appendChild(document.createTextNode(theText));

if(theClass)
theButton.className = theClass;
else

Idea #2: (the better one)

Index: js/Dom.js
===================================================================
--- js/Dom.js (revision 669)
+++ js/Dom.js (working copy)
@@ -44,8 +44,14 @@
e.className = theClass;

if(theID != null)
e.setAttribute("id",theID);

- if(theText != null)
- e.innerHTML = theText;
+ if(theText != null)
+ {
+ var m = theText.match(/^<html>(.*)<\/html>$/);
+ if(m)
+ e.innerHTML = m[1];
+ else
+ e.appendChild(document.createTextNode(theText));
+ }

if(theParent != null)
theParent.appendChild(e);
return(e);
Index: js/Utilities.js
===================================================================

--- js/Utilities.js (revision 669)
+++ js/Utilities.js (working copy)
@@ -13,7 +13,13 @@

if(theTooltip)
theButton.setAttribute("title",theTooltip);
if(theText)

- theButton.innerHTML = theText;
+ {
+ var m = theText.match(/^<html>(.*)<\/html>$/);
+ if(m)
+ e.innerHTML = m[1];
+ else
+ theButton.appendChild(document.createTextNode(theText));
+ }

[Saq Imtiaz]

On 9/6/06, Bob McElrath <bob.mc...@gmail.com> wrote:

Ok I'm beginning to really hate svn.  It seems there's no way to revert
the checked-in change I sent previously.  I have to edit out all the
changes in -r668 and -r669, by hand!  Then check in new changes that
undoes 668 and 669 and adds the below!  AARRGH!!!  Fortunately this is
a small change.  I'd hate to have to undo a large checked-in change!

I'm no SVN whiz myself, but couldnt you just have reverted your working copy to the earlier revision, made the new changes, and comitted? There definitely is a revert option, I've actually used it a few times in my all of a weeks worth of svn experience.

As for the proposed implementation, to be honest I'm not too picky. As long as we can maintain backwards compatibility, I am a happy man.
Cheers,
Saq

[Bob McElrath]

Saq Imtiaz [lew...@gmail.com] wrote:
> On 9/6/06, Bob McElrath <bob.mc...@gmail.com> wrote:
> >
> > Ok I'm beginning to really hate svn. It seems there's no way to revert
> > the checked-in change I sent previously. I have to edit out all the
> > changes in -r668 and -r669, by hand! Then check in new changes that
> > undoes 668 and 669 and adds the below! AARRGH!!! Fortunately this is
> > a small change. I'd hate to have to undo a large checked-in change!
>
>
> I'm no SVN whiz myself, but couldnt you just have reverted your working copy
> to the earlier revision, made the new changes, and comitted? There
> definitely is a revert option, I've actually used it a few times in my all
> of a weeks worth of svn experience.

I tried that by doing 'svn update -r<old>'. 'svn revert' does nothing
of the sort, just flags that there are no more conflicts.

BUT you're then not allowed to commit since your local repo isn't
up-to-date.

You have to 'svn update' first, which nukes your changes.

> As for the proposed implementation, to be honest I'm not too picky. As long
> as we can maintain backwards compatibility, I am a happy man.

Bah. Backwards compatibility is backwards!!! :-P

We do have versioning for plugins now. Can't we require plugins to
specify > 2.1 if we wanted to make some incompatible change?

[Udo Borkowski]

To implement the "<html>...</html>" feature one could use the following code:

function createTextOrHTMLNode(s)
{
    var re = /^\<html\>(.*)\<\/html\>$/;
    var m = re.exec(s);
    if (m)
        {

        var e = document.createElement("span")

        e.innerHTML = m[1];
        return e;
        }
    else
        return document.createTextNode(s);
}

With this function we can replace the code in createTiddlyButton

    if(theText)
        theButton.appendChild(document.createTextNode(theText));

by this code:

    if(theText)
        theButton.appendChild(createTextOrHTMLNode(theText));


Similar for createTiddlyElement

Udo

----------
Udo Borkowski
http://www.abego-software.de

[Martin Budden]

I think there is enough discussion around this to defer the change
until 2.2.

We really need to get 2.1 out the door.

Martin

[Clint Checketts]

On 9/6/06, Udo Borkowski <Udo.Bo...@gmx.de> wrote:

Could you use &gt; or &lt;?

I could, but then the plugin would display a "&gt;" or "&lt;" in pre-2.1 TWs and "<" and ">" in TW 2.1 and later.

Call me stubborn, but in that case your plugin could detect the TW version (thats what it's there for) and offer the old behavior.

ie:

buttonText = (version.major >= 2 && version.minor >= 1) ? "Great <b>button</b> text" : "Other button text" ;

(Untested code)

-Clint

[Udo Borkowski]

I agree.

I just wanted to make sure that we don't introduce an incompatiblity "in
the last minute" without more detailed thinking about the consequences.
So let's keep the old "text node" approach in 2.1 and discuss an
extension/change (using some "innerHTML" stuff) for 2.2.

Udo

[Udo Borkowski]

I created a ticket (http://trac.tiddlywiki.org/tiddlywiki/ticket/181) for this issue.

Udo

----------
Udo Borkowski
http://www.abego-software.de



Udo Borkowski wrote: