2016 Challenge : Week #6

If you did not, read the initial blog post announcing the challenge

This week I managed to get the challenge back to 100% success ( 6 bugs fixed in 6 weeks so far ) and recover from the failures of my vacation week. Let’s see how I did that !

#2016Challenge on @FirefoxDevTools back on tracks ! https://t.co/cPemxKFuMy improved (grouped events on detail view) pic.twitter.com/9xRMbyoClG

— Nicolas CHEVOBBE (@nicolaschevobbe) 12 Février 2016

In Week #5, my patch for Bug 1228978 was r+ed. I pushed it to TRY on the weekend and as everything was good, it landed on fx-team on Monday, and made it to mozilla-central on Tuesday. Props to early week success !

The previous week, I also began working on Bug 1232681, which aimed to properly display the script-generated animation using WAAPI ( which stands for WebAnimation API. I just figured this out this weekend, before that I thought that I was missing some US cultural knowledge as I did not understand what it was. Yeah, genius ). As the visual part of it had been approved by the mentor, I only had to write some tests to make sure my changes would not break anything. It was r+ on Wednesday, landed on fx-team on Thursday, and on mozilla-central on Friday.

After this, I was looking for a new bug to work on for week #7. Patrick Brosset pointed me to Bug 1247243. This one was tricky ! While having the devtools with the animation panel open, reloading the page will lead to a blank animation panels every now and then, as if there were no animations on page.
There were in fact a Javascript error preventing the script to go further. A message was emitted to an actor that did not exists anymore. I crippled the code with console.log and finally had a grasp of what was going on.
The devtools use a message emitter system, in which an object can subscribe to emitted messages from other objects and do stuff, like we can do with events and addEventListener in plain Javascript.
The animation panel suscribe to the “new-node-selected” message, emitted by the inspector panel. So when a new node is selected in the markup view, the animation panel only shows animations on this node and its children. And in order to display those animations, it first wipes out all the current animations being displayed.
When the page is reloaded, the markup view emit a “new-node-selected” with “null”, in order to indicate there is no more selected node, and so the animation panel release all of its animations.
Then, when the page and the markup load, a “new-node-selected” is emitted with the previous selection. You can see this coming : if the second “new-node-selected” is emitted whereas the first animation wipeout is not done, it will try to also clean up the animations, and some of them might be already killed.
I submitted a patch which does some queueing to address this. When the message is received, it wait for all the pending calls to the animation cleaning function to be over, and then only it make the call to the function. I asked for feedback for this as I’m not sure that it’s the way to go to deal with this, and there may be a better way of doing this kind of things.
Anyway this was very instructive : I used Map and Taskjs which takes advantages of Promises and generator to write async code as you’ll do with synchronous one, which is what we’ll have when async/await (ES7) is available in browsers.

I was afraid to not succeed in fixing this bug, so I searched for an “easy” one for the following week : Bug 1208204. It aims to activate the space keyboard shortcut to play/pause the animations. In fact, while working on the inspector, I tried – unsuccessfully – a few times to hit the spacebar to pause the animations, like I’m used to do in video players. I worked on it on Saturday and pushed a patch to review the same day.

Which lead me to another discovering of the week. Previously, I used to do : <pre>hg export</pre> to have my patch in a diff file, and then upload it on Bugzilla, assigning the reviewer. It was a bit tedious, I had to name my diff files well, store them in a folder on my computer, find which was the right one to upload,… And repeat this for each new patch. Now, you can directly push the patch from the terminal to ReviewBoard, which I know quite well as I’m using it at work. With a little configuration, it can be as simple as this :

hg push -f review

It then take care of the related Bugzilla, attaching the diff file and putting a review flag on it, as long as you format your commit the right way. If there are comments on the patch, you just have to ammend your commit and push it again. No more trouble dealing with files :-) . The only downside for now is that if the patch is r+ed with comments and you amend and push, it would put the review flag on Bugzilla again.

I had some time to improve my “Challenge Dashboard” too:

First, I added a brief status toolbar to display some stats about how my challenge is going. After, I fixed a minor issue in the detail view. The problem was that events on the bug ( circles ), could overlap each other if they were too close in time.

I wanted to group those circles together and find a way to display them. For doing this, I sort all the entries by date and compute the x coordonate with my positionning function. After this, I use a reduce function on the entries array, and if the distance between the current element position and the previous one is less than the circle radius, put them in the same group.

That was fun and challenging. I wanted to have a rounded line to maintain a visual consistency, and I wanted it to be split in colored-chunk. So far, I’m using rectangles for each entries, a rounded thick line to the background, and a clip-path I apply to the rectangle to have the rounded effect.

See the Pen Grouping and clipPath by Nicolas Chevobbe (@nchevobbe) on CodePen.

This week made me learn a lot, and that was what I was hoping for when I started the challenge ! I just crossed 10% of the challenge, and I will try to keep it going like this for the remaining 90%.