0018145: helpers/lyricsSearch.js breaks for..in loops in arrays

ID	Project	Category	View Status	Date Submitted	Last Update

0018145	MMW 5	General	public	2021-07-17 03:50	2022-04-30 02:02

Reporter	drakinite	Assigned To
Priority	urgent	Severity	major	Reproducibility	always
Status	closed	Resolution	fixed
Product Version	5.0.1
Target Version	5.0.2	Fixed in Version	5.0.2

Summary	0018145: helpers/lyricsSearch.js breaks for..in loops in arrays
Description	The issue Peke and Lowlander were facing in 0018084 were caused by lyricsSearch.js inserting extra functions into Array.prototype, causing for..in loops in arrays to include those functions, allowing other pieces of code to break if they use for..in loops and don't handle for those functions. This can be resolved by creating local functions _add, _merge, _map, _clean and _remove, and replacing arr._merge(params) with _merge(arr, params).
Tags	No tags attached.
Attached Files	image.png (10,094 bytes) image.png (10,094 bytes)

Fixed in build	2500

drakinite 2021-07-17 04:29 developer ~0064240 Last edited: 2021-07-17 16:24	Fixed for build 2500. Assigning to Ludek for review.

Ludek 2021-07-19 12:18 developer ~0064253 Last edited: 2021-07-19 12:19	OK, it makes sense to not override the default Array object at all and use the helper functions added by Drakinite to the lyricsSearch.js (the overriding of Array object was probably just a survival from the original MM4's lyricator script). On the other hand there is still a risk that an addon developer will override it like this. BTW: Seeing that we already overriden/added several Array properties/methods, but it is important tu use the 'enumerable: false' like this: Object.defineProperty(Array.prototype, 'insert', { enumerable: false, // so that it doesn't appear in for..in loops value: function (index, item) { this.splice(index, 0, item); } });

peke 2021-12-18 01:33 developer ~0066404	Verified 2528 Works as expected. Add Tags for fod documentation in order to warn devs on possible culprit.