#metabrainz

Maybe the _ ?

well then you know it's too old verison :D

_?

CatQuest: one day I'll visit you and force-install a newer version, even if just by your older one :p

I ignore the exception by assigning it to a variable called "_"

Yeah, I dunno - the code did seem legit, but :D

I mean I have severla versions, but the plugins i use all break :(

Maybe I can test it later too and see if I figure it out

but yes pelase come to my house nd hlpe me upgrade all the oldshit/move to waterfox etc

reosarevok: I'll sort it out, it's not fair to expect you to debug my PR

I mean, it's our job to debug and fix MB code :)

isn't that what this channel is for?

But if you have time, that's sweet

especially if you cna fix it and it works and not just regress the punycode fix

well, it's my job to submit a PR that doesn't cause browsers to crash! :)

CatQuest: those two are not really connected, I'm afraid

Although, maybe if we get this try_catch thing to work, we can *also* apply it for that somehow

oh i thought you said they were *shrug*

To at least only semi-break it like before :p

:D

They're similar in that they're both connected to new code using new URL ()

But it's different places using it IIRC

In somewhat different ways

why can't you jsut use the old URL () which worked fine? :/

:D

"new URL ()" is all one thing :)

IE11 doesn't support the URL() constructor and support was added in FF26 apparently

hench why I wrote old URL () :P

But basically, it's a thing that allows us to just cut a URL into chunks nicely

but apparently it should work since FF26

atj: hmm, maybe partial support? Our only previous use of URL() didn't work properly on 44 either

Weird

(just it broke less :D )

hm

Hmm, weird, we did

CatQuest: as an aside, running such an old version of FF is quite insecure

last time, so maybe it's punycode.toUnicode which is causing the issue

CatQuest: appreciate you have your reasons for doing so but sometimes security > features

i'll see if I can find compatibility for the punycode call

atj: and I wish I could update it but again whenever I try (I was on 30 before I managed to push all the way t o40 ,and then to 44 later) I keep hitting snags in that plugins i *use* no longer work/get updated. the big whollop now is tab groups. there *was* plugin but mozilla completely scorced all old plugin-versions before some chrome update thing and therefor nothing below idk 47 or what it is is shown

the irony is that I can probably only update to 47 48 maxx anyway because at some point Mac OS 10.7 is unsupported

(never getting a mac again)

and as an *aside* I have been told already. I *know* I can't help it. this machine is already aving a SATA disc issue so I will have to find time and money to buy a new machine anyway. it 's much easier to instlal eg, waterfox or whatnot on a new machine than trying to fix old things that (for the moment) work mostly than going through so much effort to upgrade 4-5 versions where things break anyway and isn't much more "secure"

and my features aren't "features" they are "things I use every day and I can't actually do the mb work witouth them"

really, really really my biggest with is a browser that you can change everything on, visually, that has tabgroups (if you want them), you can have any kind of menu/button/text/icon configuration, and plugins of any type

and that hey upgrade silently and *only*change*security*

don't remove features, don't move features, don't break plugins, don't "we think you'll like this slick new smooth way of doing (this thing I don't care about) but oops we also removed the other not very often sued thing (which i use al lthe time)

CatQuest: yeah I get it. Unfortunately 99% of people aren't power users and browsers are ridiculously complex, so the days of community supported versions is all but over.

change fonts, move text, make buttons bigger/smaller/wiotuth text/grayscale

and that's why yo uget me using FF44

🤷

CatQuest: can you try running some code in your console to see if it triggers the bug?

uh

you mean intentionally hanging it again?

oh. kay? i'm going ot have to finish a tihng i'm doing first though. als ohow does one run code in console? webconsole?

well, hopefully not but it could I suppose!

yes, web console / developer tools

I'll see if I can get FF44 installed first.

broweser console or web console?

web i gues sicne it has a commandline

okwhat?

CatQuest: can I PM you?

if it's large you can pastbin/irccloudbin it?

try { url = new URL("https://example.com"); } catch(ex) { console.log(ex); }

probably easier for me to copy paste too

I get SyntaxError: catch without try

or is "try" part of it?

thne i jsut get URL { href: "https://example.com/", origin: "https://example.com", protocol: "https:", username: "", password: "", host: "example.com", hostname: "example.com", port: "", pathname: "/", search: "" }

OK, that works then

could be the _ as reosarevok mentioned. thanks.

hey it din't crash my browser so i'm happy to test again!

atj: Out of curiosity, can I see the PR you're all talking about?

i think it was the try { url = new URL("https://example.com"); } catch(ex) { console.log(ex); } but

(didn't get a pm)

oh, wait pr not pm

:)

CatQuest: it's all good, you did what I wanted, thanks!

Mr_Monkey: https://github.com/metabrainz/musicbrainz-serve...

Perfect thanks :

:)

I wonder if the issue could be with `for (const param of paramKeys)`, looking at a note for caniuse:

> Prior to Firefox 51, using the for...of loop construct with the const keyword threw a SyntaxError ("missing = in const declaration").

Mr_Monkey: lol, I literally just found that!

FF 44 just gave me "SyntaxError: invalid for/in left-hand side"

that shouldn't crash the browser though

Yeah, I was puzzled about that aspect

The reason I looked into the `for…of` it is that browser stopping often comes to a bad loop somewhere

But if it throws an error it shouldn't just stop running

maybe I should just use a callback function

To be on the safe side you could wrap the entirety of that function's code in the try_catch block rather than just the `new URL(…)`

OK, I can replicate it in FF44 on Linux.

And the browser freezes?

Yes, with high CPU usage. Then the "a script on this page is screwing things up" dialog appears after a while

Apparently the cause is https://staticbrainz.org/MB/common-chunks-fbad5...

:o i went out for tea and it seems you guys have found the culprit!

possibly

the debugger is highlighting a line within "var iterate = module.exports = function (iterable, fn, that, AS_ENTRIES, IS_ITERATOR)" in node_modules/core-js/internals/iterate.js

:O

Yeah, that smells like the sort of issue that would make the browser go into a bad loop

Mr_Monkey: feels like some sort of iterator overload which is being triggered by the code I added?

Well, I think it just ties back to an issue with `for…of` in older version of FF.

-You can see if reimplementing that loop without for…of fixes the issue

[critiquebrainz] amCap1712 opened pull request #347 (master…fix): Specify rel="noopener noreferrer" while opening link https://github.com/metabrainz/critiquebrainz/pu...

atj: I'd suggest replacing the use of `Set` and `for…of` by using something like `params.keys().forEach(param =>{…})`.

That way you'll be sure to stay away from iterators altogether.

Ah, no, hang on. Almost: `Array.from(params.keys()).forEach(param =>{…})`

So ugly compared to your ES6 implementation :'/

Mr_Monkey: yes I was about to ask you, the problem I've found is that iterating over an object while removing keys yields incorrect behaviour

but this isn't documented anywhere that I've seen

I mean, it makes sense

but it's a bit of a gotcha

Hm. The plot thickens.

That's why I copied the keys

so your suggestio would work

*suggestion

Mr_Monkey: see https://gist.github.com/atj/83e3ea01f7ae9721ea2...

Note how a utm_medium param remains

Right

Yes, that makes sense. That's ironically part of the reason why Iterators were implemented in the first place :p

heh :)

But I'm guessing `Array.from(` will be a similar solution to your `new Set(…` and separate key iteration from the keys themselves

I haven't tried running any of that code, though, so I'm not certain

Mr_Monkey: thanks. I used set originally because you can multiple params with the same name, but searchParams.delete("x") removes every param named "x"

running "var s = new Set(url.searchParams.keys())" in the FF44 console just triggered the freeze :)

Actually, I wonder if you could instead loop over TRACKING_PARAMS and use `searchParamas.delete` for each key:

>If the property which you are trying to delete does not exist, delete will not have any effect and will return true

That was the likely culprit, that `Set`. I wonder if it's specifically `.keys` that it doesn't like, or any iteration of an iterator like Set.

This works for me, looks like: https://gist.github.com/atj/83e3ea01f7ae9721ea2...

[critiquebrainz] amCap1712 merged pull request #347 (master…fix): Specify rel="noopener noreferrer" while opening link https://github.com/metabrainz/critiquebrainz/pu...

Mr_Monkey: yeah I think that's the best option. I was trying to be too clever and optimise for the general case where there are very few params but it's more trouble than it's worth.

Well, your solution did look a lot better.

Mr_Monkey: thanks for your help!

Happy to!

:D