Fix subtitle download

client/src/app/shared/shared-video-miniature/abstract-video-list.ts

@@ -196,7 +196,6 @@ export abstract class AbstractVideoList implements OnInit, OnDestroy, AfterConte
     // No more results
     if (this.lastQueryLength !== undefined && this.lastQueryLength < this.pagination.itemsPerPage) return
 
-    console.log('near of bottom')
     this.pagination.currentPage += 1
 
     this.setScrollRouteParams()

client/src/app/shared/shared-video-miniature/video-download.component.html

@@ -3,16 +3,14 @@
     <h4 class="modal-title">
       <ng-container i18n>Download</ng-container>
 
-      <div *ngIf="videoCaptions" ngbDropdown class="d-inline-block ml-1">
+      <div class="peertube-select-container title-select" *ngIf="hasCaptions()">
-        <span id="dropdown-download-type" ngbDropdownToggle>
+        <select id="type" name="type" [(ngModel)]="type" class="form-control">
-          {{ type }}
+          <option value="video" i18n>Video</option>
-        </span>
+          <option value="subtitles" i18n>Subtitles</option>
-        <div ngbDropdownMenu aria-labelledby="dropdown-download-type">
+        </select>
-          <button *ngIf="type === 'video'" (click)="switchToType('subtitles')" ngbDropdownItem i18n>subtitles</button>
-          <button *ngIf="type === 'subtitles'" (click)="switchToType('video')" ngbDropdownItem i18n>video</button>
-        </div>
       </div>
     </h4>
+
     <my-global-icon iconName="cross" aria-label="Close" role="button" (click)="hide()"></my-global-icon>
   </div>
 
@@ -21,20 +19,35 @@
       The following link contains a private token and should not be shared with anyone.
     </div>
 
+    <!-- Subtitle tab -->
     <ng-container *ngIf="type === 'subtitles'">
-      <div class="input-group input-group-sm">
+      <div ngbNav #subtitleNav="ngbNav" class="nav-tabs" [activeId]="subtitleLanguageId" (activeIdChange)="onSubtitleIdChange($event)">
-        <input #urlInput (click)="urlInput.select()" type="text" class="form-control input-sm readonly" readonly [value]="getLink()" />
+
-        <div class="input-group-append" *ngIf="!isConfidentialVideo()">
+        <ng-container *ngFor="let caption of getCaptions()" [ngbNavItem]="caption.language.id">
-          <button [cdkCopyToClipboard]="urlInput.value" (click)="activateCopiedMessage()" type="button" class="btn btn-outline-secondary">
+          <a ngbNavLink i18n>{{ caption.language.label }}</a>
-            <span class="glyphicon glyphicon-duplicate"></span>
+
-          </button>
+          <ng-template ngbNavContent>
-        </div>
+            <div class="nav-content">
+              <div class="input-group input-group-sm">
+                <input #urlInput (click)="urlInput.select()" type="text" class="form-control input-sm readonly" readonly [value]="getLink()" />
+
+                <div class="input-group-append" *ngIf="!isConfidentialVideo()">
+                  <button [cdkCopyToClipboard]="urlInput.value" (click)="activateCopiedMessage()" type="button" class="btn btn-outline-secondary">
+                    <span class="glyphicon glyphicon-duplicate"></span>
+                  </button>
+                </div>
+              </div>
+            </div>
+          </ng-template>
+        </ng-container>
       </div>
+
+      <div [ngbNavOutlet]="subtitleNav"></div>
     </ng-container>
 
+    <!-- Video tab -->
     <ng-container *ngIf="type === 'video'">
       <div ngbNav #resolutionNav="ngbNav" class="nav-tabs" [activeId]="resolutionId" (activeIdChange)="onResolutionIdChange($event)">
-
         <ng-container *ngFor="let file of getVideoFiles()" [ngbNavItem]="file.resolution.id">
           <a ngbNavLink i18n>{{ file.resolution.label }}</a>
 
@@ -52,64 +65,69 @@
           </ng-template>
         </ng-container>
       </div>
+
       <div [ngbNavOutlet]="resolutionNav"></div>
 
       <div class="advanced-filters collapse-transition" [ngbCollapse]="isAdvancedCustomizationCollapsed">
-        <ng-container *ngIf="videoFile?.metadata">
+        <div ngbNav #navMetadata="ngbNav" class="nav-tabs nav-metadata">
-          <div ngbNav #nav="ngbNav" class="nav-tabs nav-metadata">
+          <ng-container ngbNavItem>
-            <ng-container ngbNavItem>
+            <a ngbNavLink i18n>Format</a>
-              <a ngbNavLink i18n>Format</a>
+            <ng-template ngbNavContent>
+              <div class="file-metadata">
+                <div class="metadata-attribute metadata-attribute-tags" *ngFor="let item of videoFileMetadataFormat | keyvalue">
+                  <span i18n class="metadata-attribute-label">{{ item.value.label }}</span>
+                  <span class="metadata-attribute-value">{{ item.value.value }}</span>
+                </div>
+              </div>
+            </ng-template>
+
+            <ng-container ngbNavItem [disabled]="videoFileMetadataVideoStream === undefined">
+              <a ngbNavLink i18n>Video stream</a>
               <ng-template ngbNavContent>
                 <div class="file-metadata">
-                  <div class="metadata-attribute metadata-attribute-tags" *ngFor="let item of videoFileMetadataFormat | keyvalue">
+                  <div class="metadata-attribute metadata-attribute-tags" *ngFor="let item of videoFileMetadataVideoStream | keyvalue">
                     <span i18n class="metadata-attribute-label">{{ item.value.label }}</span>
                     <span class="metadata-attribute-value">{{ item.value.value }}</span>
                   </div>
                 </div>
               </ng-template>
-
-              <ng-container ngbNavItem [disabled]="videoFileMetadataVideoStream === undefined">
-                <a ngbNavLink i18n>Video stream</a>
-                <ng-template ngbNavContent>
-                  <div class="file-metadata">
-                    <div class="metadata-attribute metadata-attribute-tags" *ngFor="let item of videoFileMetadataVideoStream | keyvalue">
-                      <span i18n class="metadata-attribute-label">{{ item.value.label }}</span>
-                      <span class="metadata-attribute-value">{{ item.value.value }}</span>
-                    </div>
-                  </div>
-                </ng-template>
-              </ng-container>
-
-              <ng-container ngbNavItem [disabled]="videoFileMetadataAudioStream === undefined">
-                <a ngbNavLink i18n>Audio stream</a>
-                <ng-template ngbNavContent>
-                  <div class="file-metadata">
-                    <div class="metadata-attribute metadata-attribute-tags" *ngFor="let item of videoFileMetadataAudioStream | keyvalue">
-                      <span i18n class="metadata-attribute-label">{{ item.value.label }}</span>
-                      <span class="metadata-attribute-value">{{ item.value.value }}</span>
-                    </div>
-                  </div>
-                </ng-template>
-              </ng-container>
-
             </ng-container>
+
+            <ng-container ngbNavItem [disabled]="videoFileMetadataAudioStream === undefined">
+              <a ngbNavLink i18n>Audio stream</a>
+              <ng-template ngbNavContent>
+                <div class="file-metadata">
+                  <div class="metadata-attribute metadata-attribute-tags" *ngFor="let item of videoFileMetadataAudioStream | keyvalue">
+                    <span i18n class="metadata-attribute-label">{{ item.value.label }}</span>
+                    <span class="metadata-attribute-value">{{ item.value.value }}</span>
+                  </div>
+                </div>
+              </ng-template>
+            </ng-container>
+
+          </ng-container>
+        </div>
+
+        <div *ngIf="getFileMetadata()" [ngbNavOutlet]="navMetadata"></div>
+
+        <div class="download-type">
+          <div class="peertube-radio-container">
+            <input type="radio" name="download" id="download-direct" [(ngModel)]="downloadType" value="direct">
+            <label i18n for="download-direct">Direct download</label>
           </div>
-          <div [ngbNavOutlet]="nav"></div>
+
-          <div class="download-type">
+          <div class="peertube-radio-container">
-            <div class="peertube-radio-container">
+            <input type="radio" name="download" id="download-torrent" [(ngModel)]="downloadType" value="torrent">
-              <input type="radio" name="download" id="download-direct" [(ngModel)]="downloadType" value="direct">
+            <label i18n for="download-torrent">Torrent (.torrent file)</label>
-              <label i18n for="download-direct">Direct download</label>
-            </div>
-            <div class="peertube-radio-container">
-              <input type="radio" name="download" id="download-torrent" [(ngModel)]="downloadType" value="torrent">
-              <label i18n for="download-torrent">Torrent (.torrent file)</label>
-            </div>
           </div>
-        </ng-container>
+        </div>
       </div>
 
-      <div (click)="isAdvancedCustomizationCollapsed = !isAdvancedCustomizationCollapsed" role="button" class="advanced-filters-button"
+      <div
-      [attr.aria-expanded]="!isAdvancedCustomizationCollapsed" aria-controls="collapseBasic">
+        (click)="isAdvancedCustomizationCollapsed = !isAdvancedCustomizationCollapsed"
+        role="button" class="advanced-filters-button"
+        [attr.aria-expanded]="!isAdvancedCustomizationCollapsed" aria-controls="collapseBasic"
+      >
         <ng-container *ngIf="isAdvancedCustomizationCollapsed">
           <span class="glyphicon glyphicon-menu-down"></span>
 

client/src/app/shared/shared-video-miniature/video-download.component.scss

@@ -23,18 +23,12 @@
   }
 }
 
-.peertube-select-container {
+.peertube-select-container.title-select {
-  @include peertube-select-container(85px);
+  @include peertube-select-container(auto);
 
-  border-top-right-radius: 0;
+  display: inline-block;
-  border-bottom-right-radius: 0;
+  margin-left: 10px;
-  border-right: 0;
+  vertical-align: top;
-
-  select {
-    height: inherit;
-    border-top-left-radius: .2rem;
-    border-bottom-left-radius: .2rem;
-  }
 }
 
 #dropdown-download-type {

client/src/app/shared/shared-video-miniature/video-download.component.ts

@@ -1,5 +1,4 @@
 import { mapValues, pick } from 'lodash-es'
-import { pipe } from 'rxjs'
 import { tap } from 'rxjs/operators'
 import { Component, ElementRef, Inject, LOCALE_ID, ViewChild } from '@angular/core'
 import { AuthService, HooksService, Notifier } from '@app/core'
@@ -19,24 +18,26 @@ export class VideoDownloadComponent {
   @ViewChild('modal', { static: true }) modal: ElementRef
 
   downloadType: 'direct' | 'torrent' = 'direct'
+
   resolutionId: number | string = -1
   subtitleLanguageId: string
 
-  video: VideoDetails
-  videoFile: VideoFile
   videoFileMetadataFormat: FileMetadata
   videoFileMetadataVideoStream: FileMetadata | undefined
   videoFileMetadataAudioStream: FileMetadata | undefined
-  videoCaptions: VideoCaption[]
-  activeModal: NgbModalRef
 
   isAdvancedCustomizationCollapsed = true
 
   type: DownloadType = 'video'
 
+  private activeModal: NgbModalRef
+
   private bytesPipe: BytesPipe
   private numbersPipe: NumberFormatterPipe
 
+  private video: VideoDetails
+  private videoCaptions: VideoCaption[]
+
   constructor (
     @Inject(LOCALE_ID) private localeId: string,
     private notifier: Notifier,
@@ -61,15 +62,25 @@ export class VideoDownloadComponent {
     return this.video.getFiles()
   }
 
+  getCaptions () {
+    if (!this.videoCaptions) return []
+
+    return this.videoCaptions
+  }
+
   show (video: VideoDetails, videoCaptions?: VideoCaption[]) {
     this.video = video
-    this.videoCaptions = videoCaptions && videoCaptions.length ? videoCaptions : undefined
+    this.videoCaptions = videoCaptions
 
     this.activeModal = this.modalService.open(this.modal, { centered: true })
 
-    this.onResolutionIdChange(this.getVideoFiles()[0].resolution.id)
+    if (this.hasFiles()) {
+      this.onResolutionIdChange(this.getVideoFiles()[0].resolution.id)
+    }
 
-    if (this.videoCaptions) this.subtitleLanguageId = this.videoCaptions[0].language.id
+    if (this.hasCaptions()) {
+      this.subtitleLanguageId = this.videoCaptions[0].language.id
+    }
 
     this.activeModal.shown.subscribe(() => {
       this.hooks.runAction('action:modal.video-download.shown', 'common')
@@ -83,48 +94,63 @@ export class VideoDownloadComponent {
 
   download () {
     window.location.assign(this.getLink())
+
     this.activeModal.close()
   }
 
   getLink () {
     return this.type === 'subtitles' && this.videoCaptions
-      ? this.getSubtitlesLink()
+      ? this.getCaptionLink()
       : this.getVideoFileLink()
   }
 
   async onResolutionIdChange (resolutionId: number) {
     this.resolutionId = resolutionId
-    this.videoFile = this.getVideoFile()
 
-    if (!this.videoFile.metadata) {
+    const videoFile = this.getVideoFile()
-      if (!this.videoFile.metadataUrl) return
 
-      await this.hydrateMetadataFromMetadataUrl(this.videoFile)
+    if (!videoFile.metadata) {
+      if (!videoFile.metadataUrl) return
+
+      await this.hydrateMetadataFromMetadataUrl(videoFile)
     }
 
-    this.videoFileMetadataFormat = this.videoFile
+    if (!videoFile.metadata) return
-      ? this.getMetadataFormat(this.videoFile.metadata.format)
+
+    this.videoFileMetadataFormat = videoFile
+      ? this.getMetadataFormat(videoFile.metadata.format)
       : undefined
-    this.videoFileMetadataVideoStream = this.videoFile
+    this.videoFileMetadataVideoStream = videoFile
-      ? this.getMetadataStream(this.videoFile.metadata.streams, 'video')
+      ? this.getMetadataStream(videoFile.metadata.streams, 'video')
       : undefined
-    this.videoFileMetadataAudioStream = this.videoFile
+    this.videoFileMetadataAudioStream = videoFile
-      ? this.getMetadataStream(this.videoFile.metadata.streams, 'audio')
+      ? this.getMetadataStream(videoFile.metadata.streams, 'audio')
       : undefined
   }
 
+  onSubtitleIdChange (subtitleId: string) {
+    this.subtitleLanguageId = subtitleId
+  }
+
+  hasFiles () {
+    return this.getVideoFiles().length !== 0
+  }
+
   getVideoFile () {
-    const file = this.getVideoFiles().find(f => f.resolution.id === this.resolutionId)
+    const file = this.getVideoFiles()
+                     .find(f => f.resolution.id === this.resolutionId)
+
     if (!file) {
       console.error('Could not find file with resolution %d.', this.resolutionId)
-      return
+      return undefined
     }
+
     return file
   }
 
   getVideoFileLink () {
-    const file = this.videoFile
+    const file = this.getVideoFile()
-    if (!file) return
+    if (!file) return ''
 
     const suffix = this.isConfidentialVideo()
       ? '?access_token=' + this.auth.getAccessToken()
@@ -139,12 +165,31 @@ export class VideoDownloadComponent {
     }
   }
 
-  isConfidentialVideo () {
+  hasCaptions () {
-    return this.video.privacy.id === VideoPrivacy.PRIVATE || this.video.privacy.id === VideoPrivacy.INTERNAL
+    return this.getCaptions().length !== 0
   }
 
-  getSubtitlesLink () {
+  getCaption () {
-    return window.location.origin + this.videoCaptions.find(caption => caption.language.id === this.subtitleLanguageId).captionPath
+    const caption = this.getCaptions()
+                        .find(c => c.language.id === this.subtitleLanguageId)
+
+    if (!caption) {
+      console.error('Cannot find caption %s.', this.subtitleLanguageId)
+      return undefined
+    }
+
+    return caption
+  }
+
+  getCaptionLink () {
+    const caption = this.getCaption()
+    if (!caption) return ''
+
+    return window.location.origin + caption.captionPath
+  }
+
+  isConfidentialVideo () {
+    return this.video.privacy.id === VideoPrivacy.PRIVATE || this.video.privacy.id === VideoPrivacy.INTERNAL
   }
 
   activateCopiedMessage () {
@@ -155,7 +200,14 @@ export class VideoDownloadComponent {
     this.type = type
   }
 
-  getMetadataFormat (format: any) {
+  getFileMetadata () {
+    const file = this.getVideoFile()
+    if (!file) return undefined
+
+    return file.metadata
+  }
+
+  private getMetadataFormat (format: any) {
     const keyToTranslateFunction = {
       'encoder': (value: string) => ({ label: $localize`Encoder`, value }),
       'format_long_name': (value: string) => ({ label: $localize`Format name`, value }),
@@ -176,7 +228,7 @@ export class VideoDownloadComponent {
     )
   }
 
-  getMetadataStream (streams: any[], type: 'video' | 'audio') {
+  private getMetadataStream (streams: any[], type: 'video' | 'audio') {
     const stream = streams.find(s => s.codec_type === type)
     if (!stream) return undefined