fix: Refactor evented to make mincompatable with Chrome 53 #8810

mister-ben · 2024-07-21T20:58:41Z

Description

Chrome 53 doesn't like the minified output. Specifically, it throws Uncaught SyntaxError: Unexpected token ( on the ternary in the minified version of normalizeListenArgs()

video.js/src/js/mixins/evented.js

Lines 165 to 195 in e78bcc7

    
           const normalizeListenArgs = (self, args, fnName) => { 
        
             // If the number of arguments is less than 3, the target is always the 
        
             // evented object itself. 
        
             const isTargetingSelf = args.length < 3 || args[0] === self || args[0] === self.eventBusEl_; 
        
             let target; 
        
             let type; 
        
             let listener; 
        
             if (isTargetingSelf) { 
        
               target = self.eventBusEl_; 
        
               // Deal with cases where we got 3 arguments, but we are still listening to 
        
               // the evented object itself. 
        
               if (args.length >= 3) { 
        
                 args.shift(); 
        
               } 
        
               [type, listener] = args; 
        
             } else { 
        
               [target, type, listener] = args; 
        
             } 
        
             validateTarget(target, self, fnName); 
        
             validateEventType(type, self, fnName); 
        
             validateListener(listener, self, fnName); 
        
             listener = Fn.bind_(self, listener); 
        
             return {isTargetingSelf, target, type, listener}; 
        
           };

Line breaks added for clarify

Tt = (e, t, s) => {
  var i = t.length < 3 || t[0] === e || t[0] === e.eventBusEl_;
  let r, n, a;
  return i ? (r = e.eventBusEl_, 3 <= t.length && t.shift(), [n, a] = t) : [r, n, a] = t, ft(r, e, s), yt(n, e, s), bt(a, e, s), a = _(e, a), {
    isTargetingSelf: I,
    target: r,
    type: n,
    listener: a
  }
},

Chrome would be happy with this if [r, n, a] = t were ([r, n, a] = t).

Specific Changes proposed

Refactor normalizeListenArgs() slightly to use multiple statements to coerce uglify-js to include parentheses.
Also updates uglify-js, but that in itself isn't the fix.

#8783

Requirements Checklist

Feature implemented / Bug fixed
If necessary, more likely in a feature request than a bug fix
- Change has been verified in an actual browser - Browserstack Chrome 53
- Unit Tests updated or fixed
- Docs/guides updated
- Example created (starter template on JSBin)
- Has no DOM changes which impact accessiblilty or trigger warnings (e.g. Chrome issues tab)
- Has no changes to JSDoc which cause npm run docs:api to error
Reviewed by Two Core Contributors

codecov · 2024-07-21T21:02:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.07%. Comparing base (e78bcc7) to head (7418bc2).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8810   +/-   ##
=======================================
  Coverage   83.07%   83.07%           
=======================================
  Files         120      120           
  Lines        8022     8024    +2     
  Branches     1925     1925           
=======================================
+ Hits         6664     6666    +2     
  Misses       1358     1358

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

gkatsev · 2024-07-21T22:02:55Z

src/js/mixins/evented.js

@@ -182,7 +182,10 @@ const normalizeListenArgs = (self, args, fnName) => {

    [type, listener] = args;
  } else {
-    [target, type, listener] = args;
+    target = args[0];


We should add a comment here with a link to this PR so that folks will be less likely to refactor it away.

fix: Refactor evented to make mincompatable with Chrome 53

7418bc2

gkatsev approved these changes Jul 21, 2024

View reviewed changes

gkatsev reviewed Jul 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Refactor evented to make mincompatable with Chrome 53 #8810

fix: Refactor evented to make mincompatable with Chrome 53 #8810

mister-ben commented Jul 21, 2024

codecov bot commented Jul 21, 2024

gkatsev Jul 21, 2024

	const normalizeListenArgs = (self, args, fnName) => {

	// If the number of arguments is less than 3, the target is always the
	// evented object itself.
	const isTargetingSelf = args.length < 3 \|\| args[0] === self \|\| args[0] === self.eventBusEl_;
	let target;
	let type;
	let listener;

	if (isTargetingSelf) {
	target = self.eventBusEl_;

	// Deal with cases where we got 3 arguments, but we are still listening to
	// the evented object itself.
	if (args.length >= 3) {
	args.shift();
	}

	[type, listener] = args;
	} else {
	[target, type, listener] = args;
	}

	validateTarget(target, self, fnName);
	validateEventType(type, self, fnName);
	validateListener(listener, self, fnName);

	listener = Fn.bind_(self, listener);

	return {isTargetingSelf, target, type, listener};
	};

fix: Refactor evented to make mincompatable with Chrome 53 #8810

Are you sure you want to change the base?

fix: Refactor evented to make mincompatable with Chrome 53 #8810

Conversation

mister-ben commented Jul 21, 2024

Description

Specific Changes proposed

Requirements Checklist

codecov bot commented Jul 21, 2024

Codecov Report

gkatsev Jul 21, 2024

Choose a reason for hiding this comment