My Firefox patch is now in review

Been working on Firefox ticket 677122, and I’m pretty happy with my current solution.

In my last post, I mention how I have a solution, and should have a patch up and another blog post by the next day.

I did have a patch up by the next day, but it took an extra day before I got around to the post (this post). Not a huge deal.

Anyway, I did a few other changes in the code I was touching.

I noticed a typo in a comment I was modifying, so I fixed it up.

Also, there was a boolean doHashChange being declared just outside an if block, but it was only referenced inside the if block. This was the if block I was stifling the execution of, so I also felt it was appropriate to move that boolean to live in the scope of the if. Part of what I am trying to fix in this bug is not to execute the code for anchor changes if we don’t need to. That boolean was only used for anchor changes. It was also only being set if the if statement was true, so by moving it into the if, we no longer need to check if that is valid, because if it’s false, we won’t even enter that scope.

I am pretty happy with myself at the moment. I feel this is the working, right, and fast way to fix this, but we’ll see.

Tests, and running tests are a whole new problem that I will admit still know very little about. I have to start thinking about how to test something that lives in a mediaDocument, something that has nowhere for JavaScript to live.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: