Popcorn.js down the dark path of IE8 support

Posted in open source, video, webmademovies on February 10, 2012 by scottdowne

What I mean by support is that it will be possible, but it will be a “Your mileage may vary” situation. We’ll make it possible, and show how to do it, but we won’t officially support it or test against it. It will be a separate entity that the Popcorn code does not care about.

This is due to increasing demand for Popcorn.js to reach more audiences, and while I do believe this is a dark road, and one that detrimental to the fight to kill IE8, I also find it a unique challenge…

Personally, I believe the numbers in IE8’s market share to be inflated, and not really accurate. It is the enterprise environment that refuses to upgrade to the next version, and not the home user. I would say for each home computer, there is one or more users. And, for each enterprise install, there are three more sitting in the office next to it that are only used once a week. I know this, because we have a bunch of dead offices in our building, probably with Windows XP and IE8. So now this person can view Popcorn on Fox’s website one day a week, instead of doing work…

Anyway, we found a way to have our cake and eat it to.

First, we’re pulling players out of core, and into a module. We are doing this as players are mainly for youtube support, which is flash, which works on IE8. So by pulling it out, we can make the player module 100% support IE8. Then, we are going to create a shim, that when included before a clean popcorn.js file, will make it dirty. So now you include your shim, popcorn, and attach the player module, and you’re good to go. I also considered simply including the shim inside the player module, as without the player module, the shim is pretty pointless, but I am not sold on this yet; need more discussion.

Another note. This will not mean each and every one of our plugins will support IE8. Some might, some might not. But, plugins can be modified and created with IE8 in mind, and that is the idea of a plugin anyway.


OSD 700 project release 0.5

Posted in open source on February 10, 2012 by scottdowne

So, my release for this week is a much better fix than before to bug 677122, and it is now in review.

This release started with canceling anchor code via a search in a string for what I thought was a valid case, but it turns out a string like this “#t=50,100″ is, unfortunately, a valid anchor name.

I asked for help, and was pointed to a boolean variable MozSyntheticDocument. Ended up not being exactly what I needed, as it was a protected member variable, but it started me down the path to finding the public getter function IsSyntheticDocument.

I wired it all together by getting an instance of the document as a nsIDocument, and was able to call the IsSyntheticDocument from that.

I updated a comment regarding the code I was working on, to illustrate the changes. I fixed a typo while I was at it!

My whole fix is centered around one if statement, that checked if a variable doShortCircuitedLoad is true. Basically, doShortCircuitedLoad happens if:

a) we’re navigating between two different SHEntries which have the same document identifiers, or

b) we’re navigating to a new shentry whose URI differs from the current URI only in its hash, the new hash is non-empty, and we’re not doing a POST.

Previously, I would just add my check next to the doShortCircuitedLoad check, so looked like this “if (doShortCircuitedLoad && myCheck) {” now, I added some logic in the doShortCircuitedLoad declaration, so it’s all in one place. Much cleaner.

Finally, there was a variable doHashchange, that was defined outside this if statement, it was only ever referenced inside the if, and was only ever true if doShortCircuitedLoad was also true. I moved its declaration into the if, and dropped the dependency of doShortCircuitedLoad, as we’re only ever going to touch the variable if doShortCircuitedLoad is already true.

Then put it into review.

I suspect my next task is to find someone specific to review it.

My Firefox patch is now in review

Posted in open source on February 9, 2012 by scottdowne

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.

Update on my update to my Firefox bug.

Posted in open source on February 7, 2012 by scottdowne

An update to my last post, and my bug.

Turns out, it was an update issue. My code was simply out of date, and what I needed was so new it was still hot. A good and quick lesson.

Initially after the update, I could no longer build, but some more advice led me to doing a clobber build and it worked fine. Another good quick lesson.

Also, my fix is now working and much better than it was before.

Before, I would check if the anchor string looked like a media fragment, but a media fragment string could also be a valid anchor.

My new fix simply changes the same check to see if the document is a synthetic one. Looks like this “!document->IsSyntheticDocument()”.

A synthetic document is something like viewing an image, audio, or video file in a new tab. This is also good, because there is no point in entering the anchor logic if an anchor is not valid in the loading document.

I should have a patch up tomorrow, with a new blog.

Quick update on my Firefox bug

Posted in open source on February 6, 2012 by scottdowne

So, my “working” fix for Firefox bug 677122 will not actually work.

It will potentially break any anchor names that look like media fragment code.

Example “#t=50,100” is, unfortunately, a valid anchor name. I am going to need to get more specific with my check, which is fine. I figure I would instead of doing a check on the string, do a check to see if the document is a MediaDocument, as MediaDocuments cannot (that I know of) have anchors.

I took hump’s advice, and asked for help in the #introduction channel on IRC.

I got a reply in less than two minutes from a “jdm”. I never did figure out his real name, but I thanked him as jdm.

Jdm’s advice was to check for MozSyntheticDocument. This makes sense for me. MozSyntheticDocument is a readonly boolean value attached to the nsIDOMDocument. I was able to get a reference to this document, breakpointed it, and it is confirmed to be what I want, except when I try to compile, I get an error that MozSyntheticDocument does not exist…

I searched MXR for SyntheticDocument, and found some other interesting bits. A mIsSyntheticDocument which is protected, but exposed via a IsSyntheticDocument function which is attached to the msIDocument.

I grabbed a reference to msIDocument, same deal, not there. Hmm.

Also tried a GetMozSyntheticDocument function, attached to nsDocument. Frustratingly enough, also not there…

Then it hit me what could be wrong… I have been using MXR as a reference, then go in and write my code, without double checking that MY code was up to date, and had the functionality I was seeing on MRX. Currently doing an update. Will post again soon with updates.

OSD 700 project release 0.4

Posted in open source on January 28, 2012 by scottdowne

For school, I am doing project releases in courses OSD600 and OSD700.

In OSD600 I completed three iterations, 0.1-0.3. Now in OSD700, I need to do 0.4-1.0.

This post will outline what I did for 0.4.

I’ve started by taking Firefox bug 677122 which was labeled as “not quite as easy”. I am always up for a challenge. For the tl;dr check my diff.

I will talk about what I did right, what I did wrong, and what I learned.

Three main things I did in this release are:

Created a working Firefox build on Windows. Which was not quite as easy because of bug 718541. I ended up narrowing what was needed to get a working build, and adding my findings to the ticket. Now being able to reproduce this problem, in hopes that it can be used to track down the exact cause. The ticket is now fixed.

Got a working developer environment working on Windows with Visual Studio 2010. This took more time than I wanted, as I am so used to web development where things like firebug work right out of the box. In other courses and projects, the code base is small enough that I don’t need any real developer tools, except my mind.

The real work was done in bug 677122. I had three attempts on this, and will describe each one, and why the first two failed and last succeeded.

First attempt was to wire the go to anchor code to process media fragment code. This failed because process media fragment should not be called out of context. It was originally a protected function, so that there should of been a red flag that this was going to fail. I moved it to public, then in the anchor code I would get a reference to the media element, and call ProcessMediaFragment, which would crash when trying to grab the mLoadingSrc. I suspect this is due to the way the media element lives in the media document. I loaded up firebug and actually looked at it, and it did not even have a src attribute attached to it. Probably why it failed, as it was not completely what I wanted. I did manage to parse the fragment code so I knew the difference between a fragment and an anchor, which would be crucial in the third attempt.

Second attempt was to try to get around the loading src, by doing this at a different time, or by calling a different function. Both attempts failed for similar reasons. I don’t think the timing was even the problem, but the reference to the element itself. A media document’s video element is not complete, and should not be used as such. It is a different beast. I tried things like, LoadWithChannel and resetToURI which both failed because they would access an element that was not what they expected it to be. Again, it doesn’t have a src attribute like most media elements. I also tried to do the work in the InternalLoad function, before the anchor code. This failed for the same reasons, but did get me choking off the GoToAnchor code, which helped me find the next part.

Third attempt was actually putting together two elements from the failed attempts. I moved the check for anchor or fragment to the InternalLoad, where I choked off the anchor code. It was as simple as that. Check for fragment and choke of the anchor. Once done, let the rest of the code to continue on its way. I have created a patch for this and submitted it for feedback in the ticket.

The biggest lesson I learned was a soft lesson. I should of probably asked for help between Monday’s post and Wednesday’s post. In retrospect, reading those posts now, I can see the naivety in my attempts. I was so positive about some of my solutions, and I can see it in my words. This will get better when I get more comfortable in the community, as I have been here before.

Some smaller lessons were more technical things about Firefox code. Like how to get a reference, cast it, string management, etc. These are things specific to Firefox code, and not really C++.

Warning, some of the links to Firefox source code will bitrot as the code changes and the lines where things live changes. Not sure of a better way to link to the source code.

Thanks for reading 🙂

Update on Firefox bug 677122

Posted in open source on January 25, 2012 by scottdowne

Last night I thought I made a lot of progress on Firefox bug 677122. Turns out, I just made some progress. I am building another option as I type this, so things may still be better than I think, but we’ll see.

What I have accomplished is getting a reference to the video element, inside the video document, at the time the user clicks enter while selecting the address bar that has some media fragment data. Test video link.

I also accomplished exposing the ProcessMediaFragmentURI function to this scope. This was my initial goal, and what I hoped could be the finish line. That function should be doing all I need and This is a huge step, but there is one small problem. That function crashes if the video has already been loaded.

I am still wrapping my head around why this happens. What I do know:

The existing code only calls ProcessMediaFragmentURI inside the MetaDataLoaded. So currently, this function is only ever called during a load.

So, from inside ProcessMediaFragmentURI, GetCurrentSpec is being called. It hits line 2880 and checks if something called mLoadingSrc is true. If the video is already loaded, this enters an object, and starts the process of a termination of the browser.

I have options. I can either investigate this termination, possible bug, and make the ProcessMediaFragment function work on loaded and playing videos. Or, I can figuring out how to properly call LoadWithChannel, which will call ProcessMediaFragment inside a loading context. The later is what I am currently building, as it seem like the path of least resistance. But, I suspect it may not be the best solution in the end.