First review of my first Firefox patch

So I had a my first Firefox patch 677122 reviewed. It was a review-needs-work, but that’s fine.

I had to get some help to find someone to review it. I asked in #introduction, and was told Boris Zbarsky was a good candidate, but also to check the log of the file I changed and look for previous reviewers. I saw Boris Zbarsky as a pretty active reviewer, only to cement this advice. I ran the command “hg log path/to/file > output.txt” to check the file for previous reviewers.

My patch failed because there is simply a better approach. I was reloading the page if we had a new hash in a synthetic document, but that would require the video to constantly be refreshed. He suggested I add a script to media documents to listen for haschange events, and do the work there. This is awesome to me, as JavaScript is something I am more familiar with, and I get to do some regular expression work, which I always enjoy.

I have two of the three requirements with this new method already done. Did not take much time at all to write some JavaScript to do this.

I have this:

window.addEventListener( "hashchange", function( event ) {

  var match = /#t=(\d*)(,)?(\d*)/.exec( event.target.location.hash );

  var start,
      end;

  var media = document.getElementsByTagName( "video" )[ 0 ] || document.getElementsByTagName( "audio" )[ 0 ];

  if ( !media || !match || ( match[ 2 ] && !match[ 3 ] ) ) {

    return;
  }

  start = match[ 1 ] || 0;
  end = match[ 3 ];

  media.currentTime = start;

  if ( end ) {

    var listener = function( event ) {

      console.log( this.currentTime, end );
      if ( this.currentTime >= end ) {

        this.pause();
        media.removeEventListener( "timeupdate", listener, false );
      }
    };

    media.addEventListener( "timeupdate", listener, false );
  }

  media.play();
}, false );

This is full of early return and regex awesomeness. It first sets up a listener for hash change events (I just noticed I may need to wait for the page to finish loading), next I check the hash and get a reference to the media element. I check that both of these are valid, if not, return. Then I apply the start time, and setup a listener for the end time. This may need tweaking as time goes on, but it is something to work with.

I also have the code needed to insert my script into the media document creation:

  nodeInfo = mNodeInfoManager->GetNodeInfo(nsGkAtoms::script, nsnull,
                                           kNameSpaceID_XHTML,
                                           nsIDOMNode::ELEMENT_NODE);
  NS_ENSURE_TRUE(nodeInfo, NS_ERROR_OUT_OF_MEMORY);

  nsRefPtr<nsGenericHTMLElement> script = NS_NewHTMLScriptElement(nodeInfo.forget());
  NS_ENSURE_TRUE(script, NS_ERROR_OUT_OF_MEMORY);

  head->AppendChildTo(script, false);

This gets the node info for a script node, creates a script with the node info, and appends it to the head element. This works, and am currently writing this post in my build.

The last part is to figure out how to get all my JavaScript into that script element.

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: