Nicolas Chevobbe

2016 Challenge : Week #7

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

On Monday, I already had 2 bugs going on, and I was pretty confident this week would be a success.

The previous week I started working on Bug 1247243, which aimed to fix an issue where the animation inspector won’t show any animations when the page is reloaded. I had submitted a patch with a queuing mechanism to prevent this and was waiting for feedback from Patrick Brosset.
He rightly pointed out that this is not a correct answer to address the problem. As the function is triggered when a new node is selected in the inspector view, if the user keeps the arrow-down key pressed down, the queue would grow and grow and it will cause unwanted delays in the animation inspector.
Patrick told that it should not be the client side that should handle the AnimationPlayerActors lifetime, but the server side. I followed his recommendations, but did not know how to write proper tests for this. On Friday, Patrick gave me additional informations that will help me to write the test this week.

I also asked for review on Bug 1208204. The bug was quite simple: pressing the spacebar shoud play or pause the animation in the animation inspector, just like in video players. The patch handled this well, but what I did not know is that there is a special case in the animation inspector. If the selected node or its children does not contain any animation, there is a global button that play or pause all of the animations in the page. So I implemented this behaviour, and after some back and forth with Patrick, submitted a new patch with correct tests which properly simulate key events. I am now waiting for review on this patch.

So that make 2 bugs pending, and none resolved. Do you start to worry ?

I don’t, because I do fixed a bug this week. A bug that I did not not planned to work on. Let’s rewind. The previous week, I worked on a little pen to explain how I used clip path in my dashboard.

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

I was debugging the pen using the devtools, and I had to get a clip path element in Javascript. I looked at my inspector, saw a clippath node, and typed document.querySelector('clippath')in the console. And all I got was… nothing.

clipPath issue
There must be something wrong here

Turns out, the correct name of the element is “clipPath”, with an upper P, and querySelector require a correct case to return the element.
So, as I’ve been used to do since the beginning of my challenge, I headed to IRC in order to know if it was a known problem. I had a quick chat with Julian Descottes, and he agreed that it was indeed an issue, and that I could file a bug. So did I (Bug 1248381), and I took advantage of this to work on it right away.
The fix is quite simple, I added a function to get the name of an element, and in it, it looks at the element namespace to check if it is a SVG element, and if yes, return node.nodeName, which contains the proper name. I also added some tests to it, an pushed this to review. I was r+’d by Mike Ratcliffe and the patch landed on central on Friday.

Thanks to this, week #7 is a success and I keep the streak going on ! Next week will be hopefully a success too with the keyboard shortcut bug, and I’ll go on working on the animation panel crash bug.

I also worked on something else this week, but I’ll give it a dedicated post tomorrow, stay tuned !

What a teaser !