search

NCIOCPL / cgov-digital-platform

The Cancer.gov Digital Communications Platform
GNU General Public License v2.0
11 stars 33 forks source link

PDQ videos not loading #2148

Closed blairlearn closed 5 years ago

blairlearn commented 5 years ago

Videos in PDQ content do not appear.

Go to https://www-prod-acsf.cancer.gov/types/liver/patient/bile-duct-treatment-pdq#_256 There is a gap in the content. Compare to https://www.cancer.gov/types/liver/patient/about-bile-duct-cancer-pdq#_256

This appears to be due to a JavaScript when attempting to find a non-existent objectid attribute on an element.

(Despite the paths, these are the same summary. The one on current production is part of the "Banana Split" pilot.)

bryanpizzillo commented 5 years ago

The markup for videos has changed. The source template is located at ${repo_root}/docroot/profiles/custom/cgov_site/themes/custom/cgov/cgov_common/templates/media/video/video_macros.twig

PDQ XSLT needs to be:

<div data-embed-button="media_entity_embed" data-entity-embed-display="{{TEMPLATE}}" data-entity-type="media" data-caption="" class="embedded-entity {{POSITION}}">
    <figure class="video">
      <!-- If we show title -->
        <h4 tabindex="0" class="title">{{ TITLE }}</h4>
      <!-- end if -->
      <div id="ytplayer-{{YOUTUBE_ID}}" class="flex-video widescreen" data-video-url="{{ EMBED_URL }}" data-video-title="{{ TITLE }}">
        <div class="video-preview--container" tabindex="0">
           <img src="{{ YOUTUBE_IMAGE }}" width="480" height="360" alt="">
          <div class="video-preview--play-button">
            <svg height="100%" version="1.1" viewBox="0 0 68 48" width="100%">
              <path class="play-button--bg" d="M66.52,7.74c-0.78-2.93-2.49-5.41-5.42-6.19C55.79,.13,34,0,34,0S12.21,.13,6.9,1.55 C3.97,2.33,2.27,4.81,1.48,7.74C0.06,13.05,0,24,0,24s0.06,10.95,1.48,16.26c0.78,2.93,2.49,5.41,5.42,6.19 C12.21,47.87,34,48,34,48s21.79-0.13,27.1-1.55c2.93-0.78,4.64-3.26,5.42-6.19C67.94,34.95,68,24,68,24S67.94,13.05,66.52,7.74z" fill="#212121" fill-opacity="0.8"></path>
              <path d="M 45,24 27,14 27,34" fill="#fff"></path>
            </svg>
          </div>
          <p>{{ TITLE }}</p>
        </div>
      </div>
      <!-- IF HAS CAPTION -->
        <figcaption class="caption-container">
          {{ CAPTION }}
        </figcaption>
     <!-- END IF -->
    </figure>
</div>

Example for: https://www.cancer.gov/types/liver/patient/about-bile-duct-cancer-pdq#_256

<div data-embed-button="media_entity_embed" data-entity-embed-display="view_mode:media.video_display_medium_no_title" data-entity-type="media" data-caption="" class="embedded-entity align-center">
    <figure class="video">
      <div id="ytplayer-fQwar_-QdiQ" class="flex-video widescreen" data-video-url="https://www.youtube.com/embed/fQwar_-QdiQ?feature=oembed&autoplay=1" data-video-title="metastasis: how cancer spreads">
        <div class="video-preview--container" tabindex="0">
           <img src="https://img.youtube.com/vi/fQwar_-QdiQ/hqdefault.jpg" width="480" height="360" alt="">
          <div class="video-preview--play-button">
            <svg height="100%" version="1.1" viewBox="0 0 68 48" width="100%">
              <path class="play-button--bg" d="M66.52,7.74c-0.78-2.93-2.49-5.41-5.42-6.19C55.79,.13,34,0,34,0S12.21,.13,6.9,1.55 C3.97,2.33,2.27,4.81,1.48,7.74C0.06,13.05,0,24,0,24s0.06,10.95,1.48,16.26c0.78,2.93,2.49,5.41,5.42,6.19 C12.21,47.87,34,48,34,48s21.79-0.13,27.1-1.55c2.93-0.78,4.64-3.26,5.42-6.19C67.94,34.95,68,24,68,24S67.94,13.05,66.52,7.74z" fill="#212121" fill-opacity="0.8"></path>
              <path d="M 45,24 27,14 27,34" fill="#fff"></path>
            </svg>
          </div>
          <p>metastasis: how cancer spreads</p>
        </div>
      </div>
        <figcaption class="caption-container">
Many cancer deaths are caused when cancer moves from the original tumor and spreads to other tissues and organs. This is called metastatic cancer. This animation shows how cancer cells travel from the place in the body where they first formed to other parts of the body.
        </figcaption>
    </figure>
bkline commented 5 years ago

@venglisch - Bryan said he doesn't care which of us does this, as long as it gets done. I'm starting in on the modifications. Let me know if you've already done some work on the ticket.

@bryanpizzillo - this is just a CIS problem, right? I'm guessing that the filter for DIS isn't affected, because -- even though the CDR schema would allow videos in a DIS SummarySection -- the GateKeeper XSL/T only handled video for CIS documents.

bkline commented 5 years ago

@bryanpizzillo - In the instructions above, the div/figure/div element has a hard-coded id attribute of ytplayer-jC8CUIID2HA but in the original GateKeeper XSL/T that value is derived from the unique_id attribute in the PDQ document's EmbeddedVideo element. Can you confirm that we should ignore the unique_id attribute in the PDQ document, and use the hard-coded value shown above?

bryanpizzillo commented 5 years ago

@bkline Go ahead and keep the unique ID since you have it at your disposal. HTML states IDs should be unique.

@bryanpizzillo - In the instructions above, the div/figure/div element has a hard-coded id attribute of ytplayer-jC8CUIID2HA but in the original GateKeeper XSL/T that value is derived from the unique_id attribute in the PDQ document's EmbeddedVideo element. Can you confirm that we should ignore the unique_id attribute in the PDQ document, and use the hard-coded value shown above?

bkline commented 5 years ago

@bryanpizzillo - Is it a mistake that the values to be used for TEMPLATE include _large_title but _medium and _small (without _title) for the ones that include a title?

venglisch commented 5 years ago

@bkline - Yes, at the moment this is only a summaries issue. There are currently no DIS with video on Cancer.gov but CIAT is planning to at videos to some of those documents as well.

bryanpizzillo commented 5 years ago

@bryanpizzillo - Is it a mistake that the values to be used for TEMPLATE include _large_title but _medium and _small (without _title) for the ones that include a title?

This has been corrected.

bkline commented 5 years ago

@bryanpizzillo - really hard-code language="en"?

venglisch commented 5 years ago

Well, we don't have any Spanish videos at the moment but I would use the document's language setting.

bryanpizzillo commented 5 years ago

@bryanpizzillo - really hard-code language="en"?

Actually, just remove it all together. It is an artifact generated by Drupal's Entity Embed module. Since this is not getting rendered by Drupal, it does not matter. I updated the above accordingly.

bkline commented 5 years ago

One more confirmation. The code I'm replacing has a no-resize class (in addition to the caption-container class for the figcaption element used for videos. Is it intentional that we're dropping this additional class?

bkline commented 5 years ago

@bryanpizzillo I have modified the filter to match the new markup specified in the ticket, and I can now see the video preview image in publish preview. For example, https://cdr-dev.cancer.gov/cgi-bin/cdr/PublishPreview.py?Session=guest&ReportType=pp&Version=cwd&DocId=62678#_244. However, when I try to play the video, I get a JavaScript error.

Access to XMLHttpRequest at 'https://www.youtube.com/embed/fQwar_-QdiQ?feature=oembed&autoplay=1' from origin 'https://cdr-dev.cancer.gov' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource.

bryanpizzillo commented 5 years ago

Go-Live version

@bkline This should be what you are looking for. A couple of changed from the embed from you tube's site you copied out earlier:

PDQ XSLT needs to be:

<div data-embed-button="media_entity_embed" data-entity-embed-display="{{TEMPLATE}}" data-entity-type="media" data-caption="" class="embedded-entity {{POSITION}}">
    <figure class="video">
      <!-- If we show title -->
        <h4 tabindex="0" class="title">{{ TITLE }}</h4>
      <!-- end if -->
      <div id="ytplayer-{{YOUTUBE_ID}}" class="flex-video widescreen" data-video-url="{{ EMBED_URL }}" data-video-title="{{ TITLE }}">
         <iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/{{YOUTUBE_ID}}?rel=0&showinfo=1" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen title="{{ TITLE }}" alt="{{ TITLE }}"></iframe>
      </div>
      <!-- IF HAS CAPTION -->
        <figcaption class="caption-container">
          {{ CAPTION }}
        </figcaption>
     <!-- END IF -->
    </figure>
</div>

Example for: https://www.cancer.gov/types/liver/patient/about-bile-duct-cancer-pdq#_256

<div data-embed-button="media_entity_embed" data-entity-embed-display="view_mode:media.video_display_medium_no_title" data-entity-type="media" data-caption="" class="embedded-entity align-center">
    <figure class="video">
      <div id="ytplayer-fQwar_-QdiQ" class="flex-video widescreen" data-video-url="https://www.youtube.com/embed/fQwar_-QdiQ?feature=oembed&autoplay=1" data-video-title="metastasis: how cancer spreads">
        <iframe width="560" height="315" src="https://www.youtube-nocookie.com/embed/fQwar_-QdiQ?rel=0&showinfo=1" frameborder="0" allow="accelerometer; autoplay; encrypted-media; gyroscope; picture-in-picture" allowfullscreen title="metastasis: how cancer spreads" alt="metastasis: how cancer spreads"></iframe>
      </div>
        <figcaption class="caption-container">
Many cancer deaths are caused when cancer moves from the original tumor and spreads to other tissues and organs. This is called metastatic cancer. This animation shows how cancer cells travel from the place in the body where they first formed to other parts of the body.
        </figcaption>
    </figure>
bkline commented 5 years ago

I have a new filter for CIS documents which addresses this issue, following Bryan's instructions above. To see the result, you can (for example) pull up the Pancreatic Neoendocrine Tumors patient summary from the www-dev-ac, or use publish preview from the CDR DEV server (there is a known issue which causes publish preview to fail on the DEV tier intermittently; I'm looking into that).

This is what the video preview image normally looks like:

Screen Shot 2019-07-10 at 8 39 46 AM

However, I have sometimes seen that image in a degraded state, looking more like this:

image

I have only seen this happen on Chrome in publish preview, after viewing the video and then reloading the page. I have not been able to reproduce it when viewing the page directly from the Drupal server, nor can I get it to happen in Safari. Perhaps Chrome and YouTube are doing some tracking of the video viewing activity, and this is somehow reflected in the preview image I see, though it's difficult to understand why I wouldn't also see it on Chrome when I'm not using publish preview, as the HTML markup is the same, as far as I can see. Marking the ticket as ready for IA review. If it's approved, I'll install the new filter on the other tiers, and we can schedule a re-load of the PDQ nodes for the different Drupal servers.

lburack commented 5 years ago

Hi @bkline - I'm seeing the video clearly on https://www-dev-ac.cancer.gov/types/pancreatic/patient/pnet-treatment-pdq in Chrome and Safari desktop. Seems like desktop quality is fine, but when viewing mobile breakpoints (or on my phone), the video quality state gets degraded. Once I begin to play the video, the quality is fine though. I think this passes IA review then.

bkline commented 5 years ago

@lburack - could you or someone with access to a real windows machine test this using Internet Explorer? I'm getting

image

on my DEV VM in IE (similar message on CDR DEV), though I can view the video using Chrome.

Thanks!

venglisch commented 5 years ago

I think it's interesting to note that only the video image is degraded but not the title text and the NIH logo at the top of the image.

bkline commented 5 years ago

I think the IE weirdness is OK, and is only an issue on the CBIIT Windows servers (though confirmation from an IA or tester would be good). I was able to find an old Windows laptop and bring up the page in IE and view the video.

I have installed the filter changes on all four CDR tiers. I think www-dev-ac has the fixed PDQ content (though I'm still wrestling with problems in connecting to that Drupal server from CDR, so can't be 100% certain). I've been told to keep my paws off of www-test-ac for now, so I'll hold off on fixing the PDQ on that server. Let me know which other Drupal servers need to get a fresh PDQ load and when you'd like it done.

venglisch commented 5 years ago

I ran PP in IE using my Parallels Windows 10 installation and the video works fine.

bkline commented 5 years ago

I have resolved the connection anomalies on DEV (a bug in my own code) and did a fresh push of all PDQ content to www-dev-ac. So the only remaining action items before closing this ticket are: