870709 - Rico Livegrids are not interpreting HTML coding properly since FF20

Status: RESOLVED WONTFIX

(Web Compatibility :: Site Reports, defect)

Description

[Benjamin]

Attachment: Issue FF.pdf

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:21.0) Gecko/20100101 Firefox/21.0
Build ID: 20130430204233

Steps to reproduce:

HTML coding passed by SQL query in Rico Livegrid is not interpreted anymore since FF20. Issue has also not been solved since FF21.
Interpretation works well on other browsers even on IE.

Kind of query from PHP coding:
$view1 = "<a onclick=\"javascript=view(";
$view2 = ")\" ><img width=\"16\" height=\"16\" src=\"./templates/standard/img/16x16/user_comment.png\" border=\"0\" /></a>";

$sqlglobal= "select concat('$view1',p.id,'$view2') from table p";

Also useful to know: this bug is not related to Rico or Jquery version. 


Actual results:

Since FF20, de query results are displayed in flat characters instead of displaying it as interpreted HTML items (i.e. an underlayed link to execute the mentioned js).


Expected results:

A correct interpretation of the HTML code in Rico Livegrid.
See document in attachment.

Comment 1

[Tiziana Sellitto [:tiziana]]

Thanks for reporting this. 
Do you have this problem with previous FF version?

Comment 2

[Benjamin]

Thanks for your reaction.
Nope in FF 19 and previous, it was working fine.

Comment 3

[Loic]

Regression range:

m-c
good=2012-12-24
bad=2012-12-25
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d348dbf1dab4&tochange=dc2abccc2adb

My guess goes to suspected bug:
Peter Van der Beken — Fix for bug 811701 (Move innerHTML/outerHTML/insertAdjacentHTML from HTMLElement to Element). r=bzbarsky.

Comment 4

[Boris Zbarsky [:bzbarsky]]

What are the actual steps to reproduce?  I don't see anything other than a screenshot in this bug...

Comment 5

[Loic]

(In reply to Boris Zbarsky (:bz) from comment #4)
> What are the actual steps to reproduce?  I don't see anything other than a
> screenshot in this bug...

Private testcase send by email, it's a simple testcase (just 3 clicks and observe) but I failed to find a reduced testcase.

Comment 6

[Boris Zbarsky [:bzbarsky]]

So far what I know is that simply moving the innerHTML back from Element.prototype to HTMLElement.prototype fixes the site.

That implies the bug is in some bogus assumptions the site is making somewhere, but I haven't been able to figure out where the problem is yet.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Also, if innerHTML is present on _both_ prototypes, the site is still broken.

So I tried seeing what exactly calls Get/SetInnerHTML on non-HTML elements here, and, thanks to the fact that this stuff is not minified, found it.

Here's what's going on.

In ricoCommon.js there is a function like so:

  _getEncodedContent: function(parentNode) {
     if (parentNode.innerHTML) return parentNode.innerHTML;
     switch (parentNode.childNodes.length) {
      case 0:  return "";
      case 1:  return parentNode.firstChild.nodeValue;
      default: return parentNode.childNodes[1].nodeValue;
    }
  },

which, if called on a non-HTML element, will return the text in the textnode inside it in old builds but will return the _HTML-escaped_ version of that text in new builds, since it now ends up with the .innerHTML of the element, which of course escapes things like '<' in its text content.

This is being called, in the case that really matters here, with the following stack:

0 anonymous() ["http://demo.mariquecalcus.be/include/js/rico/src/ricoCommon.js":110]
    this = [object Object]
1 anonymous() ["http://demo.mariquecalcus.be/include/js/rico/src/ricoCommon.js":104]
    this = [object Object]
2 anonymous() ["http://demo.mariquecalcus.be/include/js/rico/src/ricoLiveGrid.js":97]
    this = [object Object]
3 anonymous() ["http://demo.mariquecalcus.be/include/js/rico/src/ricoLiveGridAjax.js":196]
    this = [object Object]
4 anonymous(arguments = undefined) ["http://demo.mariquecalcus.be/include/js/rico/src/prototype.js":209]
    arguments = undefined
    this = [object Window]
5 anonymous() ["http://demo.mariquecalcus.be/include/js/rico/src/prototype.js":1308]
    this = [object Object]
6 anonymous() ["http://demo.mariquecalcus.be/include/js/rico/src/prototype.js":1238]
    this = [object Object]
7 anonymous(arguments = undefined) ["http://demo.mariquecalcus.be/include/js/rico/src/prototype.js":209]
    arguments = undefined
    this = [object XMLHttpRequest]

Line 196 of ricoLiveGridAjax.js is:

   var newRows = this.dom2jstable(rowsElement);

and line 97 of ricoLiveGrid.js (in dom2jstable) is the loop body in:

      for ( var j=0; j < cells.length ; j++ )
        row[j]=RicoUtil.getContentAsString(cells[j],this.options.isEncoded);

and getContentAsString (on ricoCommon.js line 104) does this:

  getContentAsString: function( parentNode, isEncoded ) {
    if (isEncoded) return this._getEncodedContent(parentNode);
    if (typeof parentNode.xml != 'undefined') 
      return this._getContentAsStringIE(parentNode);
    return this._getContentAsStringMozilla(parentNode);
  },

so we land in the _getEncodedContent code above.

So the fundamental problem is that _getEncodedContent is broken: for HTML elements it used to return the HTML-escaped serialization of all the kids while for non-HTML elements it just returned the raw text inside one particular textnode in the parent.  Now that innerHTML exists on all elements it returns the HTML-escaped serialization in all cases, but the callers at least sometimes expect the "raw text" behavior.

This needs to be fixed in the Rico library....

Comment 8

[Boris Zbarsky [:bzbarsky]]

I reported https://sourceforge.net/p/openrico/bugs/26/

Comment 9

[Boris Zbarsky [:bzbarsky]]

But if someone else knows a better way to get hold of the library's authors, please do!

Comment 10

[Karl Dubost💡 :karlcow]

The bug opened by Boris is still opened and had no activity
http://sourceforge.net/p/openrico/bugs/26/
for 1+ year.

The last time this project was updated:  Last Update: 2012-12-11 

I don't think anything will happen on this project.