Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tricky inheritance issue with FF 29 #195

Closed
jambonnade opened this issue May 6, 2014 · 7 comments
Closed

Tricky inheritance issue with FF 29 #195

jambonnade opened this issue May 6, 2014 · 7 comments

Comments

@jambonnade
Copy link

Hi,

We have a bug that appeared with Firefox 29 : some property in one object is lost when calling a redefined method.
The conditions to reproduce are very particular but I identified them. Maybe it may lead to identify a bigger problem.

Here is the code

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="fr" lang="fr">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
    <title>Title</title>
    <script src="http://ajax.googleapis.com/ajax/libs/prototype/1.7.2.0/prototype.js"></script>
</head>
<body>
    <div><textarea id="log" rows="10" cols="80"></textarea></div>
    <p><button type="button" id="btn">demo</button></p>
    <script type="text/javascript">/* <!-- */

        var SuperClass = Class.create({
            'initialize': function () {
                this.data1 = 'a';
                this.data2 = 'b';

                // My "fix" is to make the browser fetch the properties with
                // this, it seems the bug doesn't appear then
                //Object.keys(this);

                this.log('init');
                //this.log('data3 (0) : "' + this.data3 + '"');

                // meth1 adds data3 in SubClassA
                this.meth1();
                this.log('data3 (1) : "' + this.data3 + '"');

                // meth2 erases data3 in SubClassA (bug)
                this.meth2();
                this.log('data3 (2) : "' + this.data3 + '"');

                this.log('\n');
            },

            'meth1': function () {
                this.log('meth1');
            },
            'meth2': function () {
                this.log('meth2');
                // meth2 needs to add data in SuperClass to trigger the bug
                this.data4 = 'x';
            },
            'log': function (stuff) {
                $('log').value += stuff + "\n";
            }
        });

        var SubClassA = Class.create(SuperClass, {
            'meth1': function () {
                this.log('meth1 (sub)');
                this.data3 = 'z';
            },
            'meth2': function ($super) {
                // meth2 needs to be redefined to trigger the bug
                this.log('meth2 (sub with super())');
                $super();
            }
        });

        var SubClassB = Class.create(SuperClass, {
            // Whatever here
        });


        $('btn').observe('click', function () {
            // Need to init another object from another subclass to trigger
            // the bug.
            new SubClassB();
            // Bugged object :
            new SubClassA();
        });

    /* --> */</script>
</body>
</html>
@jambonnade
Copy link
Author

I forgot to mention : doesn't happen if firebug (or dev console) is opened. I believe firebug accesses the created objects in some way ; that's why, i added "Object.keys()" to fix, it may do the same kind of stuff internally.

@Yaffle
Copy link
Contributor

Yaffle commented May 6, 2014

@Gruikmusic , interesting bug, did you post a report at bugzilla.mozilla.org ?
i can reproduce this without prototype.js :

<script>
function SuperClass() {
  this.meth1();
  var z = this.data3;
  this.meth2();
  if (z !== this.data3) {
    alert('bug!');
  }
}
SuperClass.prototype.meth1 = function () {
};
SuperClass.prototype.meth2 = function () {
  this.data4 = 'x';
};
var F = function () {
};
F.prototype = SuperClass.prototype;
function SubClassA() {
  SuperClass.call(this);
}
SubClassA.prototype = new F();
SubClassA.prototype.meth1 = function () {
  this.data3 = 'z';
};
SubClassA.prototype.meth2 = function () {
};
function SubClassB() {
  SuperClass.call(this);
}
SubClassB.prototype = new F();
new SubClassB();
new SubClassA();
</script>

@jambonnade
Copy link
Author

i prefered to let you (prototypejs contributors) report to mozilla when found whether it's related to firefox or prototypejs

@Yaffle
Copy link
Contributor

Yaffle commented May 7, 2014

@Gruikmusic , so you have to wait prototypejs contributors to do this....
Anyway, i think, it is not good, that common constructor creates different properties on an object, because of redefined "meth1" and "meth2"...

@jambonnade
Copy link
Author

I reported to mozilla https://bugzilla.mozilla.org/show_bug.cgi?id=1008339

About derived methods in constructor, I'm used to do this in PHP and I find it interesting :

class Parent
{
  public function __construct($params)
  {
     // [...]

     $this->_initSomeStuff();
     $this->_initOtherStuff();

     // [...]
  }
  protected function _initSomeStuff()
  {
    // default code
  }
  protected function _initOtherStuff()
  {
    // default code
  }
}

_initSomeStuff and _initOtherStuff may be overridden then in subclasses to customize some parts of initialization while keeping a common construction (and a common constructor signature)

@Yaffle
Copy link
Contributor

Yaffle commented May 9, 2014

I reported to mozilla https://bugzilla.mozilla.org/show_bug.cgi?id=1008339

good.
;

About derived methods in constructor, I'm used to do this in PHP and I find it interesting :
Thats OK.

But in javascript the sequence of property assignments affects the "class" of an object.
So, it is better to define all properties in the constructor.

@savetheclocktower
Copy link
Collaborator

Looks like Firefox fixed this, which is good, because I wouldn't have known where to start. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants