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.

Fixes #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 (106b84a).
Report is 2 commits behind head on main.

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.

src/js/mixins/evented.js

Co-authored-by: Gary Katsevman <git@gkatsev.com>

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

src/js/mixins/evented.js Show resolved Hide resolved

add comment

58225de

mister-ben added the patch This PR can be added to a patch release. label Jul 22, 2024

gkatsev reviewed Jul 22, 2024

View reviewed changes

src/js/mixins/evented.js Outdated Show resolved Hide resolved

gkatsev approved these changes Jul 22, 2024

View reviewed changes

Update src/js/mixins/evented.js

106b84a

Co-authored-by: Gary Katsevman <git@gkatsev.com>

mister-ben merged commit a7c9f26 into videojs:main Jul 22, 2024
11 checks passed

This was referenced Jul 23, 2024

Update Vite configuration in zimui for native ES6 support openzim/youtube#272

Merged

Upgrade video.js version to fix issue with minification on Chrome 51-59 openzim/youtube#275

Open

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 •

edited

Loading

codecov bot commented Jul 21, 2024 •

edited

Loading

	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

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

Conversation

mister-ben commented Jul 21, 2024 • edited Loading

Description

Specific Changes proposed

Requirements Checklist

codecov bot commented Jul 21, 2024 • edited Loading

Codecov Report

mister-ben commented Jul 21, 2024 •

edited

Loading

codecov bot commented Jul 21, 2024 •

edited

Loading