GitLab Pair Programming - Frontend, 30 Jul 2021

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: FE Pairing - 20210730 - Special observers & special characters

Description

In this session we pair program on a preexisting MR around improving MutationObserver's in our Jest test environment. We then pair on fixing a bug around usernames with special characters in MR assignees.

MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67250

A

Recording and progress yeah, so we are talking about this problem, why my mac starts freezing and cannot boot up, um because now this is the problem. This is the problem right like no. I have from time to time that it just freezes when you want. You know when it has a sleep, wake failure, but please ignore the low stack shot. What I mean, why is.

B

A

Okay, cool, um no uh all kidding aside, so we want to release a major upgrade of gitlab ui, which uh drops browser support for some older browsers, which means we don't need to polyfill one older one, one browser ap. I called the resizeobserver we already fixed this in gitlab ui, it's fixed in a branch there, but tests started failing in gitlab as well with every component that uses the uh the observer right under the hood, and so the problem is gitlab, not gitlab.

A

But what what do I want to say? So? The problem is that jsdom, which is the mock implementation under just hood: right, doesn't ship the resize observer and we have a mock implementation of observers already.

A

um It has some caveats, though, and I'm going to take you through it and uh tell you what I found and how I solved my problem and then we can do something else, and I.

C

Already solved the problem.

A

Kinda, I still have some questions on what you think right, so I'm just starting to share my screen.

A

Is this the right screen? Do you see my browser or see the browser cool so.

C

The you might have a sleep wake issue somewhere, though.

A

Yes, you know the stack trace there. So what's really interesting.

A

The current implementation of our mock observers just support one mock observer, and so all the things that we do on the one mock observer, you know if we decide to mock a certain observer, will just happen on the last one that we've built so in case and what we actually have in the code in on the test. If we, you know accidentally, create multiple observers, we didn't trigger, like the observation on all the observers and we actually created like too many observers and so yeah.

A

I modified this code to um you know, handle multiple observers and just iterate through them and just have one trigger function. We also had this thing where we counted the observers, um which um isn't really the thing that we actually wanted to test in a test that did it in the test that did it. We wanted to see that you know stuff gets properly disconnected, so I changed the tests to spy on the observer disconnect and you know hey that shouldn't have been called okay, we're destroying.

A

It has been called one time right, but now the fun thing is suddenly tests started failing um in the jest integration tests and I'm like what the heck happened, um because I chose with the resizeobserver. This is the observer where we dropped the polyfill and suddenly a lot of tests start failing and the fun thing is. We also have a lot of tests that actually don't do anything with the mock intersection observers. We just need to add them so that the tests don't fail, because some underlying components are creating observers right.

A

um So you can see here like we're using a mock-up server, even though that democrat, so we're not doing anything with the mock-up server. So my thought is hey. Why don't we add like all the observers globally and we're good to go and if we need to override them with our mock? That does you know more things.

A

We can use the existing mock-up server and finally, tests started failing and it all comes down to this uh wait. I should share my code, I uh I commented this out again: I'm logging all the observers and it seems jstorm, actually implements a proper mutation observer, so we actually just need to mock the other three, because we actually have a proper mutation of cell cool yeah.

A

So that's the fun thing, and uh now I was trying to.

D

Say sorry when, when you say js dom implements a proper mutation observer, you mean actually works like a musician observer. Should.

A

So and that's that's actually pretty funny, because I just fixed a test like a normal jess test, because I thought wait. Oh god give me the last commit here that I didn't push yet.

A

What is happening? Okay, so I already fixed this one spec and in this spec we have a mutation observer on this blob viewer element right and we actually looking for mutations on the attributes and with my changes this spec failed.

A

So I now manually trigger the mutation observer right, the mock mutation observer and then the tests work again, but this all just happened because I overwrote the original observer right. So in order to test my theory that I now fix things with, you know this: let's not mock the object, mutation observer all right.

A

Let's start this one spec again and I'm going to increase font size here, a little bit right.

A

Cool, please yeah. It's passing, I'm going to revert or no just reset file to state before commit right, boop cool, so tests probably should still be passing. Let's double check that the file actually has been, which one is it table, content spec yeah. It has been reverted to the old one right.

D

It's probably unnecessary, can you kill and restart the jest process? Yes, uh it actually does it automatically, but it was off screen. So I didn't see it. Okay,.

A

Cool the spec should now be failing because we created the environment like an overrode demutation observer, yes, and so, if we fix that again.

A

It seems like we need to restart when we change the environment right.

A

It actually works so yeah the fun. The fun thing was we need. We don't need to overwrite a mutation observer here, right, cool cool.

A

So it's really funny because this, I think you know make we already had the performance observer here as a no op observer by the way, and the only thing is we're adding the intersection observer and the resize observer, so anything that relies on them doesn't need like a manual mock anymore, which I think is an improvement.

A

The only downside I could see is like what happens if jsdom actually starts implementing these um yeah.

C

Hopefully, that'll be like a change log or something, and we can.

A

D

Hopefully, what we could do here is we could do a check in the environment setup so that, if, if it's already defined, then we don't override it, but we maybe log a warning.

C

It's a great idea: yeah.

D

A

So you're basically saying uh for.

A

C

For loop man, yeah.

C

D

A

Sorry too bad too much bash right. uh If this global observer.

C

Well, we don't need to throw something right like then we just peacefully move on right.

C

I mean, if it's already defined. Doesn't that mean that we're just like with mutation observer, we're good.

A

Overwrite and existing js dom observer. Let me just.

C

Oh, I misinterpreted. I thought that let's just only set it if it's not already set, I mean yeah. That also would be fine right, like maybe we can counsel warren just like. Oh, we already found this we're not going to do anything, but then, if we don't find it we'll set it.

B

So how likely is anyone to ever see this console warning.

A

Yes, that's why I want to wanted to throw so in case you update, js, dom. You actually need to think about what you want to do when you update js, dom yeah.

D

Yeah, I think either works. I think when you say you know who would see it. I think it would be obvious to anyone as soon as you run, a you know focused just spec like like we did just now. That's true, but throwing, I think, is yeah fail early, fail fast.

A

Oh and then we just do this global.

A

Observer equals no op cool.

D

Can we throw a new error rather than a string.

A

A

Cool: what's the implications.

B

Of the difference.

D

uh Throwing a string um has no stick trace, yeah, exactly you constructing a new error, builds a stat, trace and and so on.

A

Cool, so what we can do here now is, we can just add, like a mutation observer, to see that this works and how this would work right, awesome, yeah and oh geez. We need to restart it right.

A

It's fine, it's pretty funny, so it actually means that you yeah it. We override an existing observer in jsdom. Are you sure you want to do that? Do that right?

A

I mean it's actually funny, because now the question is like: do we even need the normal mock mutation observer anymore, because you can just use the real observer? You can just do the real mutation right.

C

um So correct me if I'm wrong, but the mock observer is something that can be used for any one of those intersection. Mutation, resize right.

A

um It currently can just be used for intersection and mutation observer, because that's the only ones that we.

C

Probably don't we probably don't need the mach one and yeah yeah, and so the question probably don't need the mutation one. So.

A

Yeah so- and it's actually funny because.

C

Well, it looks like we use like triggering it and that's.

A

Yeah and that's what I meant right so rather than mocking and triggering it, we actually could trigger the mutation that should cause the thing to you know like, for example, if you go into the pop overs you could probably I mean what are we checking here again uh here? We checking that the observer actually disconnects right um yeah, I don't know I mean, maybe maybe a future pairing session can think about this um yeah.

D

I think what's written here what's written here, looks fine, you know we're just spying on the observer. We don't care if it's mocked.

A

Yeah, that's actually true right, so oh wait trigger mutate, so we must do a mutation here. Anyhow,.

D

Okay, so maybe we could do it.

A

Here, for example, instead of triggering mutation, we could actually just um what do are we doing here removes pop over when target is removed from documents, so we could just remove the target from the document right, yep, yeah, that's where it's worth the follow-up for sure I mean it would be interesting uh to check this, or do you feel this is out of scope?

A

I feel like it's out of scope, because you just want to work on a different thing right.

C

Yeah, um no, I feel like it's out of scope, because the main goal is upgrading git, lab, ui and actually.

A

It's not the main goal. I I my main goal is to get the integration branch passing and because this observer shenanigans is completely independent from the upgrade kinda, we could actually pull it out into a new merge request, merge the mutation, observer testing, changes and then go forward with our lives right then I'd upload, update, gitlab, ui right, but like this test, wouldn't isn't failing right.

A

So yes, this test isn't failing, but that's what I just wanted to see what actually happens if we I I was just you know, I see I think it's gonna be like hey remove nodes. So what happens? If we uh no uh target? I hope it's a normal. You know and then you know remove.

A

I don't know man I mean if they have a proper implementation in jsdom. This should just work right, yep.

C

Does that really just work yeah, it did. Can you really.

B

Comment yeah: can you comment that out yeah if little faith.

C

Wow there you go.

B

Yeah, yes, so so it's kind of a tech debt for someone to go and pull out all of the unnecessary use of the mutation mock mutation, observer right.

A

Yeah, essentially, what we're saying here instead of mocking some dom api, we can actually use it because js dom implements it right and it's.

C

A

Preferable to the other stuff.

C

Yeah no, but I yeah, I wouldn't do it all. In the same, mr, like.

D

No, no, no! No! No! No! No, definitely not, but there are three specs files which use the use mock mutation, observer.

A

Is that yeah, all right and and another one is tooltips, so it's exactly the same spec, but with tooltips instead of pop overs and the other one is the image lazy, loader, um oh yeah, yeah, and in in the image lazy loader, we mock the intersection observer, which we need to mock but yeah it. I think it would still make the tests a little bit. I don't know how often are we triggering the mutations here trigger child mutation?

A

So what is trigger child mutation yeah so actually funny? Because, probably if we weren't overwriting the in this lazy loading spec, this should also work. But I agree with you folks that this is probably a follow-up and it will be a neat, mr just removing the mockumentation observer, because we don't need it anymore. Pop a little good.

B

For new contributors and a little apple on it, and someone in the hackathon can pick it up. Yeah yeah yeah.

C

You can edit that yeah for sure.

A

And create an epic for it yeah exactly, but.

B

Free free issues from three to the spanx yeah.

A

Cool exactly no. I was wondering because the fun thing how I detected it um by the way is we're using this testing library called testing library uh for the integration specs. It seems to be similar to capibara, query stuff right and it's funny because it uses mutation observers under the hood and I'm just like how did this ever work in jsdom, because if it's using them under the hood, you know in order to wait for elements and these kind of things. How did that work? Because we just have the mock implementation?

A

Anyhow right, yeah, surely, testing library.

D

Has like a full back it can't. No, it does strictly rely on mutation observer. It's really surprising. No.

A

I mean it's basically like the weight for implementations in capibara. Right, like you, add a mutation observer that says hey, you know, I'm waiting for that element. Wait for whatever right once the document body changed and an element is there and give it a timeout of two seconds. I mean yeah.

D

um I'm just surprised that it doesn't have like you know if mutation observer exists, use mutation, otherwise just poll yeah or something.

A

What what is paul doing, uh yeah.

A

Okay, cool um spend the first 20 minutes on that. We'll push the changes while we're talking about other things: um cool.

C

Yeah that was awesome. I also shared an eye chat, a link to um so I looked up on sourcegraph repos, with jsdom and mutation observer um you're lovingly sent to their uh mutation observer implementation, which looks like there's a commit from like two to three years ago. That included this. So but I guess maybe we were in a much older version of js dom when we first had to do the mock observer stuff. So that was interesting cool thanks for sharing that ip.

C

That was really cool, and it's cool to see that we'll be able to test this and more of an integration. um Does anyone have anything else that they want to appear on? I I have a wealth of things that we can just pick from, um I kind of like doing testing stuff, but I don't know if anyone uh there's also, we could check out the is this known channel and just pick something that seems broken.

B

Oh, do you know what's been bugging me and I think paul it's out of here. I think.

A

You're involved.

B

We should really rename this call to lee codes with good levels. This.

A

B

This isn't there's a main one. Now, actually you know the um the next badge in the top left hand corner for the last. What three months that's been appearing, big and then shrinking? Why does it do.

D

Yeah, it has to be that's related to the recent. um What is called the application utilities file getting loaded too late, or something like that. It's css load ordering. Basically, oh.

C

I have another one that actually.

D

Gets similar, it's also part.

B

C

Css too um yeah.

B

Oh yeah was it the work you were doing to slim it down optimize it. I can't remember what it was exactly yeah. We.

A

Can um but we can, we can fix it, definitely uh yeah. I I I have another one that you know relates to me. Actually, oh okay, and it's one. That's really annoying wait. Let me share my screen. I don't know if you want to work on that. One well.

C

I I do want to fix. I I'm uh you know. I appreciate you bringing that up late, because it has bothered me for a little while uh this next splash.

B

I don't know why that ship over us very.

D

C

Though, and- and I can't totally fix it- oh.

D

C

We gotta fix this one.

A

I feel like that was fixed at one point and it got broken again. We need to know the fun thing. Is it really depends on which components are used. So if, if it's just me, it's fine, but if it's that list- and we have like and also it's not necessarily about the merge, it's also about the busy indicator, because.

B

A

With that one, because.

B

I had that on my list as well.

A

Yes, so I don't know.

C

To be honest, I don't know if it I don't know if it relates to this one asterisk. I think.

A

I don't know, do you want to do both.

C

Let's do both of them.

A

Who wants to drive my? I don't have a gdk running.

C

um I can drive. That's fine. uh Let me let me just clear out some things um which one do we want to do.

B

C

B

If, if you're, um let's do the um apostrophes and funny characters first, but if you're good at multitasking paul, what's your take on why the the next badge is? Is it was it mark or someone's to do with what they said about the order of loading, or, I think.

C

That's prob, that's related, but it's also related to startup css, where we are loading initially just the critical css to get the top bar and the left sidebar um to show up as soon as possible. But yes, so.

D

C

D

Go ahead. Sorry, sorry, paul! um I guess the issue is that we're doing startup css, but not in a canary scenario. Right, that's gonna, be it.

C

And- and that's all we have to do is update our fixture to flag to force that next badge to show up and it'll be caught, so that'll be really easy to do. Okay, um making people that have apostrophes and quotes in their name uh that that'll be the harder one.

B

More important, though you know how unloved must you feel to exactly how.

C

B

Does that be, is that it's not always been like that all right.

D

I feel like I remember that being for years. Oh really.

B

Climate, I mean, I think it was.

D

Fixed and then got broken again right.

A

Yeah it, I think it's because we're concatenating strings uh merge, request escape.

A

This is the issue by the way uh I'm sharing it in the zoom.

C

Chat. Okay. Yeah! Let me let me hop in that issue, so I'm going to share my screen. I am going to.

C

You sent me a zoom chat. Where did all the zoom stuff go? Oh, my gosh. Okay.

A

We can't see it so we can't help you it's over.

C

Here, no, it isn't at least in the screenshot. Oh, I think zoom. I think it'll hide like yeah. Something like that. I actually.

B

C

The code where it's broken- oh you did you wrong. Man.

B

So not for today, but I did a pairing session earlier in the week and I actually used live share for the first time. Pretty awesome, really yeah the vs build live, show yeah, exactly yeah.

B

That meant that we could sort of multiple people could bash at the same time, but it gets a bit confusing if you put it on follow mode, so you can see where the other person's scrolling and did you did you, try it on the gitlab repo or have you tried it on uh yeah, getting up yeah wow.

C

That's really cool.

A

So this is a so maybe go on master because I think the busy indicator has been added after okay, uh like uh the link uh has like a specific commit. That's quite old. I don't know when I I raised that issue. Over a year ago, um oh tooltip see generate assignee tooltip, oh we externalized it, so it might mean that we can fix it in one place for all the places. That's.

C

Nice, uh do I get a source graph, gonna work for me that doesn't seem to be working for me. Oh, am I uh anyways I'm just going to open this up and vs code.

C

Guitar link boom and we have generate assignee tooltip is coming from this function.

C

And is it, are we saying it's the sprint f, that's potentially causing the issues. Let me get the gdp running and we can play around.

A

I think it's this printf to be honest, because it's doing escaping right, if no it's not doing escaping. Maybe the usernames here are already escaped and depending where they're coming from, they could be escaped or could not be escaped and then depending on how we use them in view right like it's.

C

Yep, okay, we're gonna uh we're gonna, try to see if we can tdd this and create the passport uh so not sure where the name root is coming from uh user data mock. Okay.

C

So let's just create a new test for this.

C

I'll create a new describe with uh user, with a special user yeah and all.

E

C

Special really, but.

C

With a special user and I'm.

C

All right we're gonna, create a component.

C

And so we're looking for tooltip has name is a thing. I guess it's got to be true user. We are going to extend the data mock, but the name is going to be something like. Oh, I don't know lucas ip.

C

Like that or is it single quotes, it's probably single quotes, isn't it yeah? It's single quotes, and I didn't spell your first name right.

B

Yeah be single because um david's one I've noticed as well with.

A

B

A

So so the big question, though, is, is this thing that goes into the thing already escaped, and that's the reason why it's like that or you know that's the question.

C

While the gdk is starting, I was gonna see if we can recreate the bug here, cool go. It sets tool tip with unescaped special characters, man, it's going so slow for me. ah I don't know. What's going on.

A

uh Maybe we need the live coding so that we all can type in that line right. Sure, that'll.

C

Help speed it up, find tooltip text.

C

2B um should be your name, so I'm actually or at least contain your name. So I'm going to put a special name here. Lucas you have a special name.

A

Okay, I'm going to search for a fictional character that has some apostrophes, that's very good. I like keeping your name in there. You know nah. I don't like that. Oh seems like personality. I don't know.

C

Oh you're, right, uh if you wrote this and you're, if you wrote this code, it would seem a little uh what what do you think of conan? Oh barbarian, oh conan,.

A

Barbarian, I mean, don't you know, conan the barbarian, the.

C

A

Barbarian yeah.

C

A

C

That there was a special character in his name.

A

No, that's why I said like conan, oh and everyone expected brian and then barbarian. You know.

C

I think I have like a copy of the hobbit around I'm sure that there's some special lord of the rings characters not.

A

That tolkien or his uh you know susan, is talking already public domain. We need to find someone who's here.

C

I'm sure it is, I don't think so. I think that was the 30s or 40s all right. It's not 100 years. Yet I've.

A

All right, we are loading. Martha's vineyard is good.

A

Something like that, oops sorry.

C

What do you think of uh rabbit o'hare, nice, nice.

C

So freaking slow man.

B

C

B

Are you on a laptop or well yeah, but it's and is the is the fangirl mentor? Is it a thousand degrees.

C

Well, it's not, I I kind of think he's got an old machine. He doesn't suffer from that. I know. That's there you go, uh it just has taken its time, but this, I think, sentinel one, even though they've improved some of it. I I've noticed with.

E

C

Antivirus or some annoying.

A

Thing like that right, I've noticed, so I have the theory that sentinel one is trying to um fight infection like your body by just increasing the heat right like like.

C

That's pretty good. That is really good. um I clearly have broken this, but it's passing.

B

But this is so that's suggesting that it's injecting the the it's already escaping them before they get this far. Can you show me what the fine tooltip.

D

Text method is doing, function is doing yep looking for title, so it's just inspecting the attributes.

A

Oh inspecting the attributes could be problematic instead in case vgl, tooltip escapes them later on.

C

Yeah, let's I think the gdk should be close to running. So let me start.

C

C

All right: well, that's still thinking about its life choices. um Oh it's ready! Now!

C

Here we go here. We go.

A

Oh yeah, not waiting for that redirect.

C

Nope, no, that takes way too long.

A

I wonder if this other special characters, though or just a past apostrophe.

D

That's a good question: yeah. Can you check in some um german umlauts well angle brackets as well? Okay,.

C

Let's, let's create a user that that is a special user. I think you can just.

A

Rename your uh root to you only just rename the real sure.

C

Yeah I mean edit profile here. You.

A

Can just name it root, root, root, where's, my uh full name, administrator.

C

Okay, so rabbit or dough in it. What what kind of what kind of names are you.

A

C

We're looking for a rabbit with a with a ooh with an omelette what they called the.

A

Omelette of an omelette we need, no, I, I don't think you need to add an umlaut uh um is uh the german ones uh when it's like and putting more.

C

So I put any more special characters here.

A

No, it's fine for now, okay, like because, if we fix that escaping issue, I mean we need to double check after we fix this, that nobody can inject script whatever right like, so you can maybe rename yourself to rabbit or hair script or like an empty.

C

Div tag or yep yep yep, all right so here is mrs assigned to me. This is showing up that mrs or issues ms, because.

A

Mrs, have the I mean, probably also issues, if you add the busy indicator, so you would need to add the busy indicator as well.

C

A

Issues and yeah needs to have multiple, assignees. Okay, we are spinning.

A

I always wonder whether the animations of the spinning are synced.

C

They appear not to be oh, they are, they do appear. Are they or am I just looking at them wrong? Are they oh, my my eyes can't move that fast?

C

um Okay, so we got the assignees, but this only happens if there's multiple signings, yes, so we're gonna, add oh I'll. Add that and ezra here.

A

Yeah and they cannot merge so if bess or f yeah and now you need to change yourself to busy said status.

B

I didn't realize that was a condition for it to happen. Actually.

C

Yeah, I'm gonna be also a in the bathtub.

B

Yeah, that's all I was gonna suggest.

C

Oh and the bathtub that's getting good.

E

Yeah all right busy.

A

So now you probably need to reload.

C

Unless this was just fixed recently, do you think this might have just been fixed? I'm on a recent mastery.

B

I think it's been open, so long. I'd be very surprised if it had you'd be surprised if it was fixed.

A

I mean you, you can do a git log on that avatar link assignee avatar, not on the on the helper function. Oh.

C

Yeah, but that this is like a timeline of the file, and so I see the last change was two months three months ago. No.

A

Okay, then it's not fixed so yeah, probably not.

C

Yeah here we go: oh yeah, that's that's the ticket. Let's.

E

Create our failing.

C

Test so somehow we need to mark ourselves as busy that's coming from status information. All you.

A

Can said, cannot merge to true like or can merge to false if you see it on the user, like the create component thing, yeah.

C

A

C

Set can merge to false.

A

C

Oh yeah, look here: we've done! Oh yeah, yeah yeah, I see status. Information is something we're building like this got it um so yeah, I said, can merge defaults so over here when we were doing our test case. We'll just do can merge to false so.

A

Really really really stupid theory here. Why are we even using sprint f, because the translation, it makes no sense, like a good question that that's like.

C

A

Are we even sprint f in it, yep.

C

Yep, unless there is a language that, like expected, like those things to show up, I.

A

Think that you can switch what uh swap them around. I mean the whole translation here. Is you probably just need to write the four cases right or like hey, cannot merge and busy.

C

But I think something immediate I mean we could do that kind of fix, but I think for the immediate one. Maybe we just want to stream.

A

Regression right like like what like, if someone comes in and actually fixes the translation, but you then end up with a regression again, I have the feeling that the escaping is had happening somewhere down the tooltip route.

D

No, I think it's, I think it's the s printf, because um petty default is it's escaping stuff right, yeah exactly and I think it's transforming the uh apostrophe to the um to the hex entity. Yeah.

C

D

So this isn't working.

C

B

B

Is it worth us looking in the browser to see if the the value that's been passed in has already been escaped before that's a.

D

Good point: we.

B

D

Use the view defutils right, I've had a look, and the title attribute actually has what's displayed is exactly what's in the title attribute. Basically, so it's already 39 semicolon in in the in the dom.

A

So so it's just printf. Definitely, okay,.

C

So I'm I'm having a hard time creating a a test for this. uh I'm here calling create component, but.

B

Are you saying that is the name? Is the name property is still okay, then it hasn't been escaped at that point.

C

Yeah, it's this test looks like it's still passing, which means well, that's what suggests it's not the sprint. F, though right, so it's not even showing it's not even.

A

Oh, you need to change the issuable type as well to merge request. I believe I don't know if this is like. If you can, the cannot merge is basically issuable type and this user can merge. So if the issuable type um so.

C

Maybe it's easier to set the availability to busy oh you're right, here's the issue, we'll type ah you're right, you're, right, you're, right, you're, right, you're right! I didn't notice that. Thank you. Thank you.

C

Are you on the wrong test right? I am thank you no worries. I mean it's early for you right. It's nine! It's like nine, we'll type, not time uh yeah,.

D

E

Is early for you.

C

So I've I've picked up a really bad habit. This week um I've started playing chess and I started playing um bullet chess, which is one minute timer um and I'm so bad at it, but I get so addicted to it. It was like every time I lose, I'm like. Oh get me another one, and so I I I've been staying up really late playing bullet journal.

A

C

How does this thing.

A

Special name so we're using the name from the user right. Can, you just add an availability of busy as well. Just for just so we're talking about yeah, ah it's can underscore merge.

C

D

You thank you. We should we should. We should do one of these pairing sessions where we get typescript into.

A

We should do one of these sessions where we just uh fix all the under the snake case too yeah.

C

All right, so, let's, let's.

A

Fix this sucker? Okay! So let's, let's try the easiest one in the function in that helper function. Can you just change this printf to be a simple concatenation just for yeah, so.

C

We're talking about return.

C

C

uh Let me move this to its.

C

A

Probably I you could just return status information map, yeah, that's fine! Otherwise you could have just prepended a name to the status information.

C

ah That's true: I get a status information print parenthesized prepended join. Yes, that's a good something.

A

C

A

Cannot merge yeah, it looks correct now.

C

So what we're concerned about is, if I'm, you know a uh script if, on the script kind of.

A

No, I don't think we're concerned about that, because I hope that our tooltip stuff is just showing not showing html here right yeah. It should be pushing.

B

It into well is it v, safe, html or whatever else.

D

B

D

Should be doing, it should be doing just a uh text content rather than yeah generation. So we you feel like there's, not a like xss.

D

Let me look into the bootstrap view code, because that's what it's eventually going into so I'm going to.

C

Actually move this over to our parameterized specs um yeah, I'm going to move this over to our parameterized spec, to where I can actually set the name here.

C

And uh that should be nice, so, let's, let's do that? Can we just change.

B

C

Just change: why don't you just change root? uh Well, I think grab it over here. Well, I was wanting to test out. I was one to test out a funky xss case. uh I just.

B

Wanted to do just a one or two others, um but then how are you gonna know what it ultimately does with it, or are you expecting it to convert.

A

Yeah, because it's just checking the title attribute right, you.

B

Don't know what it does.

A

Under the hood.

B

Yeah, so I I don't feel like this would be the place to check that. Well,.

C

If the title attribute does not include the fancy.

B

Characters things.

C

I think it probably would.

B

Include the fancy characters, but then somewhere down the line is preventing them from being rendered as html yeah.

A

Given that we actually, if you're looking at the right, can you scroll down where we're actually using it? um um Yes,.

C

I'm saying um and I'm fine removing this, but what we, what were you looking at? What scroll down here.

A

E

A

Because we're just using where are we using the tooltip title right? Isn't I mean so one way to access srs would be, if view would be stupid, but view would is not stupid. So if you're like using double quotes here, it doesn't break the thing right.

D

A

The other way how we access us here is, if vgl2 to render stuff as html.

D

Right so I've just had a look at that and bootstrap view. uh You have to opt in uh with an 8.html modifier to the tooltip directive, so.

A

You would need.

D

A

It to vgl minus tooltip.html.

D

And so because we don't do that, it should just be uh text. Content would.

C

We like to include a test for that or not really.

B

The test wouldn't be in this component.

A

Spec, though right well, I would rather add a linter that nobody uses the html thing yeah. I.

D

C

Say exactly the same thing.

A

That's great, okay, all right! That's a.

C

Good follow-up by the way yeah yeah, uh but I do want to have.

C

Rabbit, oh gosh,.

C

Is that do they capitalize the it's two capital letters right, yeah, yeah, yeah, yeah, yeah yeah, oh gosh,.

A

It's also funny that we have this comp, it's a very complicated test to test that few strings are concatenated right.

C

A

Oh, that's not how you.

C

Do it it depends, you could make name an object good. I could make all this an object which I kind of kind of want to, but I'm not going to um it's.

A

Funny because I always come back from this tabular markdown, whatever syntax, to just writing a list of objects, because I ain't like you get too big.

C

Yeah, if it's too big I'll, do like I'll do an array with objects like with input and expected, but if it's, if it fits, I kind of like it, you don't like it. I know you don't.

A

No, I just said that I don't like it so it's fine yeah, okay,.

E

uh What what happened, what happened.

C

Oh, this one failed this one and the last one failed yeah.

A

You can drop the last one.

C

Right, like it's good, we've done the thing I wanna because I haven't. Oh, I'm actually still running a test. That's what's going on! Oh gosh! I hope it doesn't do this like forever tested.

C

C

C

I'm fine, if we just say sets tooltip.

C

Are you all? Okay with that.

C

Why is it complaining? I don't know why.

D

C

D

Mismatched, oh no, you don't!

D

C

A

Just mines, I think so here I would also complain. Why is this a describe each instead of it each? Let's do.

E

C

You want to make.

A

It each no, I don't care it's fine. It makes it a little bit more reasonable, reasonable, readable, yeah cool. Are we good okay? So my question: is you fixed it now right? Could you go back to your browser and check whether the drop down is also fixed? I reckon not right. So that's probably a different bug like if you go there and search for yourself yeah. It's.

A

I wonder where that is coming from.

B

I missed it. You already checked that it's rendering correctly, that we have fixed it in the other place.

C

C

um Yeah, let's, let's check that out: um if we have some time, I might be able to fix it.

A

I don't know if we have time to fix it, to be honest, like we're looking to replace all the drop downs in the coming milestones anyway. So yeah.

D

It might be that.

A

Is that for reals? Yes, this is mark's new job starting august 16th, replacing drop downs.

D

It just so happens that my middle name is drop down. Isn't that lucky? Yes,.

C

Mark drop down florian, that's pretty dope.

A

It's because how often do you drop down as a kid that your middle name.

E

D

Down, if only I could remember it's just my memory is so poor. I don't know why.

A

What is this, this is not view. Oh, this is snap.

E

A

Do you know, do you know this file? I just showed it to mark today. uh Can you search for user select? Yes, skittle users select, brace yourself right, like it's, it's user select index js something not view it's index. It's like a java user, select javascript yeah, there's a spec node, it's not the one.

A

Where is it it's like in a innovation.

D

A

Already fixed it.

D

No, it's so if search for users select index.js right.

C

That sh, that really should bring it up. Yes ah yeah, so this is like used to be conscript right. This is one of those.

A

Files that yes and it actually has two different implementations- I have a bootstrap bootstrap, select or select two, depending on the options you give it yep.

C

um Yeah, so here's the other interesting thing, so you did talk about. Do we want sprint f to be doing this kind of escaping thing.

A

Yes, we want to do it because otherwise, if we just for example in this example, would plainly add it, you could end up with xss but like if someone, because user names are inherently dangerous because you don't have control over them right.

C

Yes, but yeah yeah and don't.

A

C

Don't we have new utilities for like doing smart sanitization, that's what I was wanting instead of just yes, we have them in view. We don't have them here, but well.

D

Function right, we we do have uh dom purify.

C

Right, that's what I'm saying is rather than doing just a naive escaping. Maybe we want sprint f to do a purify.

A

In this instance, I would want us to replace the drop down and just delete this whole file, because it's 853 lines long and nobody knows what it's doing, and it has 18 different parameters at the top, like anything that you change in this file can lead to a regression right like look at this yeah like.

C

Yep we we did a, uh we did a. I reviewed, mr of this, and I was like this is so fragile to change. So this is one of the first penning tests that I did at gitlab was because you just can't tell am I gonna break something or not, so we just uh penned everything made some small change and then, thankfully the pens were good but yeah.

C

This is, it is gnarly and one of the things if I there's like some lines of code, because this did come from coffeescript, which is kind of like ruby, where, like the last line, is technically a return, there's some lines of code that are like just an expression that doesn't do anything, but it's meant to return it. That's something.

A

C

In like this kind of yeah, it is wild, but I do want to fix the issue you know like right now I mean this is a fairly big issue. Like sorry paul did you fix it in the other.

B

So now, when you hover over the icon, it's fixed, does it.

C

Well, I don't think I've refreshed the page.

A

B

Should have been.

A

Represented, I know the.

B

Test fixed, but sometimes yeah, I want to say it yeah. That's.

A

Actually, a good point, I don't I don't know if it's fixed. It could still be that there's like a double escape happening later on right. That's.

C

True, that's true.

A

That would be funny, though, like us talking about this big user select and we haven't fixed the fix the or original issue in the first place. Yeah.

D

It's probably getting answered go ahead. If there was if there was a double escaping, I think we'd see ampersand yeah, that's true! Oh.

A

D

A

Yeah I mean you know, I don't care about the drop down to be honest because I always use the markdown quick commands. um So I'm happy if that is fixed, and I think it's two different issues you wouldn't want to fix them in the same amount.

A

I yeah. I know it's two different issues I want to fix it I mean you can go to. We can just remove all of the escaping that we do everywhere anywhere.

C

A

Then I mean xss are good, because some security researchers can make money right like.

C

So exactly no totally and yeah, I think that's the thing is just keep over correcting. If there's a bug, then over correcting it the other way, and then we all keep our jobs so.

D

So xss is a form of wealth generation. Is that what you say? Oh yes, no.

A

Wealth redistribution right like from uh from a large private company yeah, so this is already oh. This is already double escaping right and escape is even the wrong use here. Right.

C

A

Because we actually want to probably deal.

C

With dom purify right, we have the function; let's just try it out. Can I just try dropping an n? Yes,.

A

Please try it out like just remove the two escapes and add like a dom purify around it all right. So.

D

Where's don purify live is that just uh it's, it's tilde, slash lib, slash, sanitize html, possibly okay, I'm gonna! Look for it over here.

A

No, that's the wrong one.

C

Yeah, that's just what's like.

C

Well, let me see what this one references. Maybe this one has oh, is this: this is it.

C

A

Can just call sanitize it's import sanitize from slash lip slash, not not this one. Just uh yeah lip don purify is the folder okay.

C

A

C

A

C

Because it has.

A

Some default configs and stuff right like and you just import sanitize. This is what I can import. Oh right, yes,.

C

Let's do it, let's do it. Where do I gotta go? uh I guess I go here you just how do I? How do I alphabetize things? Okay,.

D

I regret I, I think we should pull out the alphabetization of imports that really irritates everyone. I.

A

uh Yep you know I I would. I would like to add an import rule that, by the way, replaces the two dots with utility, because it's the it's shorter um right, like oh, you mean like here, because we are still going to the root yes and I would love if that would be consistent. That's more important than the order for me, but to be honest, like eslint can fix it right.

C

Here's the thing why I would rather us not try to make imports pretty, because if you don't have a lot of them, it shouldn't be a big deal and if you have a lot of them, that is a signal. Your component is probably doing too much or your module is doing too much, and it's.

D

Like or that you probably want to you know how it should be structured yourself better. I saw an instance of that the other day.

B

Right, you want to group things together in a more logical fashion, yeah exactly.

C

Right uh we, where were we, I'm looking for.

A

Oh, that's a good question. Like drop yeah, you actually had another function that also had like a list right, like format result obviously, and render row are two different things. Can I.

D

A

I suggest, rather than doing this, just import.

D

Sanitize, as uh escape.

C

Are they are they? Are they the same, probably nope? Well, they didn't do this.

B

A

Using them the same.

C

That's why I'm wondering like in this case, are we using them the same.

A

So actually, the usage of escape here is wrong like this, because escaping is escaping some things, while sanitizing and sanitizing things.

C

Like we were using escape to sanitize right, correct, okay, but you're, saying let's just do sanitize as escape and drop the escape from kodesh yeah all right. I will commit this in a separate, mr and then we can see it.

E

C

Specs fail right: all the specs fail. Man that'd be wild. If we had a spec that was asserting yeah yeah.

A

You're right, no, no, you can end your sentence there. It would be wild if we had a spec it'd, be wild.

C

You know I think this one has does user select. Have I think it does? That's the craziest part. Oh, does it? Oh you better believe it does there. It is. Oh, it also has has an.

A

Ee version yeah yep wait for it yeah.

D

Yeah, when the spec more than the implementation, that's a bad sign.

B

Much much more.

A

Oh, yes, it's like a tenth in length right.

C

I think I may have may have been. Oh, I was yeah I I was. uh I was responsible for adding the spec because it had no specs and then we were running to issues I was like. We got to do some sort of thing here, uh but because there's an ee version, which I thought evan evan, you committed that right because it says you yes, you're right: no, no! No! Okay! It's not finished loading. What's wrong. With this.

D

I mean it's an even you even you committed this all.

C

Right so we want to see it. I know come on.

A

Then I want to call my wife to you know fetch me for dinner: oh new episode of ted lasso, nice.

A

C

Oh, it might be because I have the console open. So it's like disabling cache, I don't know so. I need to probably turn it off and on my computer it gets so slow at times.

A

No, they definitely not synced. The rotational thingies. Definitely.

C

Not synced definitely not synced, they kind of they all kind of look synced to me.

A

But maybe it's the zoom zoom thing, the the compression of the.

C

Oh, maybe that's interesting. Oh, come on.

A

To be honest, like we were calling escape, we were calling escape there quite a lot, so it could be that dom sanitize now runs like 20 times on that one line right like for for each line or something like that, and so that we actually have like a performance regression here. Yeah dom purifier is quite a lot heavier than just a plane escape yeah.

B

Well, that's too bad, maybe were you were you you weren't intending on that as the the final solution, though right you just kind of wanted to to smash that in to see if it fits yeah, yeah.

C

I think um I'll create mr with a to-do question about you know, but maybe we don't want to hit that performance bit and maybe we want to just create selectively.

B

Replace a few of the escapes maybe or something.

A

Yeah, that will probably be probably good enough. If you take yes, you know, remove all the escapes and wrap in each of those functions that uses them wrap the whole html and it don't purify sanitize.

C

Well, would that work, because.

A

D

The only thing is, what might happen in that case is someone might construct um a username which breaks the rest of the template, that's being built, so I guess it could break that, so someone could have a username. That means.

A

That no one after that rendered.

D

But I mean I I think the thing is like lucas is right. Let's just not worry about it, because me, elena or mike, is going to be tearing turns to pieces, throwing it away soon. Hopefully,.

C

I think an immediate user win would be cool, but hopefully one. This is super easy and.

A

Just doesn't hurt.

C

Performance like we.

A

Could just add a database constraint that usernames are not allowed to contain a quote.

A

Yeah I mean we by the way fun way right. We have a user.

A

We had a constraint that your username is not allowed to end in a mime type like we have a constraint that your username is not allowed to end with html, because we had like browser issues with that, and people could construct weird things and we went a little bit overboard and excluded all the mime types, and we also had a mistake before where we didn't check for dot mime type, but just for the mime type so like people like that are named asimov right.

A

We're not correct usernames anymore, um but now azi.mof is not a correct. Username anymore, like us.

D

I don't know what was the actual problem we were trying to work around. I think that you.

A

Are can you turn off the recording, then I can tell you. Oh all right. Peace out.