Improve documentation for archive.org video link embeds

Add comments to clarify several issues raised during PR #554.

Also add a missing test case and a small cleanup for the function.

The conversion of start time and end time path params into query string
params is still not particularly robust but it is good enough for us to
do some initial testing in production.

src/sidebar/media-embedder.js

@@ -101,11 +101,24 @@ var embedGenerators = [
     return null;
   },
 
-/** Match Internet Archive URLs
+  /**
+   * Match Internet Archive URLs
+   *
    *  The patterns are:
- *  1. https://archive.org/embed/WHDH_20151121_043400_The_Tonight_Show_Starring_Jimmy_Fallon?start=360&end=420.3
- *  2. https://archive.org/details/WHDH_20151121_043400_The_Tonight_Show_Starring_Jimmy_Fallon/start/360/end/420.3
- *  The patterns are invariant, start and stop are always present.
+   *
+   *  1. https://archive.org/embed/{slug}?start={startTime}&end={endTime}
+   *     (Embed links)
+   *
+   *  2. https://archive.org/details/{slug}?start={startTime}&end={endTime}
+   *     (Video page links for most videos)
+   *
+   *  3. https://archive.org/details/{slug}/start/{startTime}/end/{endTime}
+   *     (Video page links for the TV News Archive [1])
+   *
+   *  (2) and (3) allow users to copy and paste URLs from archive.org video
+   *  details pages directly into the sidebar to generate video embeds.
+   *
+   *  [1] https://archive.org/details/tv
    */
   function iFrameFromInternetArchiveLink(link) {
     if (link.hostname !== 'archive.org') {
@@ -113,14 +126,19 @@ var embedGenerators = [
     }
 
     var groups = /(embed|details)\/(.+)$/.exec(link.href);
-    if (groups) {           // group 1 is 'embed' or 'details'
+    if (!groups) {
+      return null;
+    }
+
     var path = groups[2]; // group 2 is the path
+
+    // Convert `/details` paths to `/embed` paths. TV News Archive links put
+    // the start & end times in the paths whereas the embed links always use
+    // "start" and "end" query params.
     path = path.replace('/start/', '?start=');
     path = path.replace('/end/', '&end=');
-      // the iframed url is always the embed flavor
+
     return iframe('https://archive.org/embed/' + path);
-    }
-    return null;
   },
 
 

src/sidebar/test/media-embedder-test.js

@@ -92,7 +92,13 @@ describe('media-embedder', function () {
 
   it('replaces internet archive links with iframes', function () {
     var urls = [
+      // Video details page.
+      'https://archive.org/details/PATH?start=360&end=420.3',
+
+      // TV News Archive video details page.
       'https://archive.org/details/PATH/start/360/end/420.3',
+
+      // Embed link generated by the "Share" links on the details pages.
       'https://archive.org/embed/PATH?start=360&end=420.3',
     ];
     urls.forEach(function (url) {