Bug 212449 - Javascript engine gets confused, calls method on wrong object.
Summary: Javascript engine gets confused, calls method on wrong object.
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: Safari Technology Preview
Hardware: Mac macOS 10.15
: P2 Critical
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-27 17:47 PDT by MikeB
Modified: 2020-05-27 22:04 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description MikeB 2020-05-27 17:47:52 PDT
It appears that Safari is getting confused on the proper object method to call when ES6 classes are being used with inheritance. It works fine in Chrome and Firefox, but I wasn't able to put together a minimal test case for you, so instead I direct to you our app where you can reproduce it in 30 seconds, and the exact line where things go wrong.

URL: https://demo.timetrex.com/
Username: demoadmin1
Password: demo

Once logged in, click "Payroll" in the blue menu along the top of the screen, then click "Tax Wizard". Notice how the screen is blank. Click the red "X" icon at the bottom right to exit out and start over again.

I have traced the issue down to PayrollRemittanceAgencyEventWizardStepHome.js on line 50 where $this.render() is called inside a callback. If you place a break point on that line and inspect $this, its a "PayrollRemittanceAgencyEventWizardStepHome" object, which is correct and with the debugger you can even call methods that only exist in this class like $this.getNextStepName(). The PayrollRemittanceAgencyEventWizardStepHome class extends WizardStep class, and WizardStep has the render method that should be called on that line, and is on both Chrome and Firefox. 

However when you step into that line on Safari v13 or Technology Preview downloaded today, its calling the completely wrong PayrollRemittanceAgencyEventWizard.render() instead which in this case intentionally does nothing and of course nothing is rendered.
Comment 1 Yusuke Suzuki 2020-05-27 20:09:16 PDT
> $this
< PayrollRemittanceAgencyEventWizardStepHome {cid: "view247", el: k, tagName: "div", events: Object, $el: k, …}
> $this.__proto__
< PayrollRemittanceAgencyEventWizardStepHome {on: function, listenTo: function, off: function, stopListening: function, once: function, …}
> $this.__proto__.__proto__
< WizardStep {on: function, listenTo: function, off: function, stopListening: function, once: function, …}
> $this.__proto__.__proto__.render
< function render() {
		this.initCardsBlock();
		return this._render();
	}
> $this.render
< function render() {
		//do render stuff
	}
> $this.hasOwnProperty('render')
< true

So, WizardStep#render is not called because $this has its own "render" function which is empty. And it is defined in PayrollRemittanceAgencyEventWizard.js.

So, this is not related to ES6 class.
Comment 2 Yusuke Suzuki 2020-05-27 20:10:25 PDT
This "render" function is defined at this point.

initialize (TTBackboneView.js:24)
initialize (WizardStep.js:24)
(anonymous function) (backbone-min.js:967)
TTBackboneView (TTBackboneView.js:17)
WizardStep (WizardStep.js:20)
PayrollRemittanceAgencyEventWizardStepHome (PayrollRemittanceAgencyEventWizardStepHome.js:12)
Eval Code (Anonymous Script 1 (line 1))
(anonymous function) (Wizard.js:224)
(anonymous function) (Global.js:1605)
initStepObject (Wizard.js:219)
initialize (Wizard.js:45)
(anonymous function) (backbone-min.js:967)
TTBackboneView (TTBackboneView.js:17)

It seems that options["render"] is defined.
Comment 3 Yusuke Suzuki 2020-05-27 20:37:27 PDT
This is https://bugs.webkit.org/show_bug.cgi?id=38970

Since Object.render is defined as non-enumerable property, for-in for PayrollRemittanceAgencyEventWizard is listing "render".

You can avoid this issue if,

1. you define render etc. in Object with enumerable: false.

Or

2. If options is truly a shallow option object, you can change

TTBackboneView#initialize

	initialize( options ) {
		//Convert options object to this object properties as early as possible.
		if ( options && typeof options == 'object' ) {
			for ( const property in options ) {
				this[property] = options[property];
			}
		}
	}

to

	initialize( options ) {
		//Convert options object to this object properties as early as possible.
		if ( options && typeof options == 'object' ) {
			for ( const property in options ) {
                                if (options.hasOwnProperty(property))
				    this[property] = options[property];
			}
		}
	}
Comment 4 Mark Lam 2020-05-27 20:53:18 PDT

*** This bug has been marked as a duplicate of bug 38970 ***
Comment 5 Yusuke Suzuki 2020-05-27 21:08:35 PDT
(In reply to Yusuke Suzuki from comment #3)
> This is https://bugs.webkit.org/show_bug.cgi?id=38970
> 
> Since Object.render is defined as non-enumerable property, for-in for
> PayrollRemittanceAgencyEventWizard is listing "render".
> 
> You can avoid this issue if,
> 
> 1. you define render etc. in Object with enumerable: false.

Object.prototype.

> 
> Or
> 
> 2. If options is truly a shallow option object, you can change
> 
> TTBackboneView#initialize
> 
> 	initialize( options ) {
> 		//Convert options object to this object properties as early as possible.
> 		if ( options && typeof options == 'object' ) {
> 			for ( const property in options ) {
> 				this[property] = options[property];
> 			}
> 		}
> 	}
> 
> to
> 
> 	initialize( options ) {
> 		//Convert options object to this object properties as early as possible.
> 		if ( options && typeof options == 'object' ) {
> 			for ( const property in options ) {
>                                 if (options.hasOwnProperty(property))
> 				    this[property] = options[property];
> 			}
> 		}
> 	}
Comment 6 Yusuke Suzuki 2020-05-27 21:38:56 PDT
(In reply to Yusuke Suzuki from comment #5)
> (In reply to Yusuke Suzuki from comment #3)
> > This is https://bugs.webkit.org/show_bug.cgi?id=38970
> > 
> > Since Object.render is defined as non-enumerable property, for-in for
> > PayrollRemittanceAgencyEventWizard is listing "render".
> > 
> > You can avoid this issue if,
> > 
> > 1. you define render etc. in Object with enumerable: false.
> 
> Object.prototype.

No, TTBackboneView.prototype.__proto__. This one has non-enumerable render etc. And it is inherited by PayrollRemittanceAgencyEventWizard.
Comment 7 Yusuke Suzuki 2020-05-27 21:40:00 PDT
(In reply to Yusuke Suzuki from comment #6)
> (In reply to Yusuke Suzuki from comment #5)
> > (In reply to Yusuke Suzuki from comment #3)
> > > This is https://bugs.webkit.org/show_bug.cgi?id=38970
> > > 
> > > Since Object.render is defined as non-enumerable property, for-in for
> > > PayrollRemittanceAgencyEventWizard is listing "render".
> > > 
> > > You can avoid this issue if,
> > > 
> > > 1. you define render etc. in Object with enumerable: false.
> > 
> > Object.prototype.
> 
> No, TTBackboneView.prototype.__proto__. This one has non-enumerable render
> etc. And it is inherited by PayrollRemittanceAgencyEventWizard.

This is Backbone.View.prototype.
Comment 8 Mark Lam 2020-05-27 22:03:53 PDT
Re-opening to investigate what looks like a networking issue that manifests with this test case.
Comment 9 Radar WebKit Bug Importer 2020-05-27 22:04:29 PDT
<rdar://problem/63704739>