#542 ✓ staged
cadecairos

Facebook plug-in incorrectly handles "type" option

Reported by cadecairos | May 30th, 2011 @ 11:34 AM | in 0.7

The Facebook plug-in expects different "type" values upon creation to specify the type of widget you see. These values are currently: "like", "like-box", "activity", "facepile", "comments", "live-stream" and "send". If for example I pass the plugin "like_box" instead of "like-box", the plug-in will return prematurely and not populate anything in the target div. The problem with this approach is that the start and end methods still get called for that empty plugin item.

Here's a little test case I created to illustrate the problem. I pass the facebook plug-in method a type of "like_box" so that it returns early, and added alerts to start and end to show it is being called still. see it here

I've been playing around with it and I figure the best solution is to have it default to the like button if there is an incorrect type specified. Is there a better approach? such as throwing an error? I have a fix for it I just need to set up a branch and commit it.

Comments and changes to this ticket

  • cadecairos

    cadecairos May 30th, 2011 @ 11:54 AM

    • State changed from “new” to “peer-review-requested”
    • Assigned user cleared.

    Okay I set up a branch and fixed the bug by changing the return line to an assignment of "like".

    here is the commit

    here is the branch it's on

  • Scott Downe

    Scott Downe June 2nd, 2011 @ 04:25 PM

    • Assigned user set to “Scott Downe”
  • Scott Downe

    Scott Downe June 7th, 2011 @ 03:26 PM

    • State changed from “peer-review-requested” to “under-review”
  • Scott Downe

    Scott Downe June 7th, 2011 @ 03:39 PM

    • State changed from “under-review” to “review-needs-work”
    • Assigned user changed from “Scott Downe” to “cadecairos”

    I don't think the tests here are really testing anything but that something exists, yet there are four tests doing that, so basically we have four of the same tests:

      popped.exec( 2, function() {
        ok ( document.getElementById( "likediv" ).innerHTML.length > 0, "No type specified. Like button added to div" );
        plus();
      });
      
      popped.exec( 3, function() {
        ok ( document.getElementById( "likeboxdiv" ).innerHTML.length > 0, "Incorrect type value defaulted to like button" );
        plus();
      });
      
      popped.exec( 4, function() {
        ok ( document.getElementById( "activitydiv" ).innerHTML.length > 0, "Activity feed is added to div" );
        plus();
      });
      
      popped.exec( 5, function() {
        ok ( document.getElementById( "facepilediv" ).innerHTML.length > 0, "Facepile is added to div" );
        plus();
      });
    

    A quick look at the innherHTML of the facebook, shows some unique info "<fb:like" that you may be able to regex for.

    First test that there is something there, don't use length, just use the string, as an empty string is falsy.

    Then, test the type using a regex on the string, looking for key info.

  • cadecairos

    cadecairos June 13th, 2011 @ 11:12 AM

    • State changed from “review-needs-work” to “peer-review-requested”
    • Assigned user changed from “cadecairos” to “Scott Downe”

    After checkout out the html that Facebook returns when the plug-in is called, It became clear that the only differences between each plug-in type was CSS and JavaScript, with no uniquely identifying attributes we can check against reliably. I did modify the tests so that they check against the string and not the length.

    Here are the modified tests

  • Scott Downe

    Scott Downe June 13th, 2011 @ 01:47 PM

    • State changed from “peer-review-requested” to “super-review-requested”
    • Assigned user cleared.

    Tests passing in ff and chrome.

    Tests make sense and are not repetitively testing the same thing.

  • Rick

    Rick June 15th, 2011 @ 12:03 PM

    • State changed from “super-review-requested” to “review-looks-good”
    • Assigned user set to “Jon Buckley”

    Tested: facebook plugin test suite;

    Passing in:

    • FF 3.6, 4.x, 7.x

    • Chrome 12, 13, 14

  • Jon Buckley

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Pages