[Kss-devel] Bug in kukit.dom.parseHTMLNodes

[Balazs Ree]

On Mon, 05 May 2008 20:05:28 +0200, Godefroid Chapelle wrote:

> Christophe Bosse wrote:
>> I've found a bug while using replaceHTML kss command. The deal here is
>> to create a DIV element to let the browser engine to parse HTML code.
>> The problem is that in gecko browser (maybe others too) when adding a
>> node requiring a special parent node (LI, TR, TD, DT, DD etc...) it
>> will skip those nodes because the DIV  element does not fit in this
>> case.
>> 
>> Here's my dirty quick patch:
>> 
>> var restrictedParentNodes = {
>>     'th': 'thead',
>>     'td': 'tr',
>>     'tr': 'table',
>>     'thead': 'table',
>>     'tbody': 'table',
>>     'tfoot': 'table',
>>     'li': 'ul',
>>     'dt': 'dl',
>>     'dd': 'dl',
>>     'option': 'select',
>>     'optgroup': 'select',
>> };
>> kukit.dom.parseHTMLNodes = function(txt){
>>     var firstNode = /<([^> ]*)[> ]/.exec(txt); firstNode = firstNode ?
>>     firstNode[1] : null; var node =
>>     document.createElement(restrictedParentNodes[firstNode]
>> || 'div');
>>     node.innerHTML = txt;
>>      console.log(node.firstChild);
>>     // gecko engine automatically adds a TBODY node if
>>     (node.firstChild) {
>>         if (node.firstChild.nodeType == 3)
>>             node.removeChild(node.firstChild);
>>         if (node.firstChild.nodeName == 'TBODY')
>>             node = node.firstChild;
>>     }
>>     var resultNodes = [];
>>     for (var i=0; i<node.childNodes.length; i++) {
>>         resultNodes.push(node.childNodes.item(i));
>>     }
>>     return resultNodes;
>> };
>> --
>> Christophe BOSSE - Développeur
> 
> Thanks for the report !
> 
> I think your fix looks nice !

I find one particular issue: maybe the first text node should not be 
removed unconditionally. Can you explain in more details, why this becomes
necessary?

In certain cases we do want the first text node to appear, it is allowed
to add something like this to an ordinary tag like <P>: 

   "Text <b>bold</b> and <b>more bold</b> is accepted."

I have the feeling that your patch would remove "Text" from the beginning.

But maybe this is not allowed anyway from where you do want to remove 
them, so the removal can be used conditionally.

(Jeroen: I think we have to add tests for these cases to the list in the 
blueprint.)


-- 
Balazs Ree