Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Video Trimming: Avoid crash due to missing tracks property #12556

Merged
merged 3 commits into from
Nov 3, 2022
Merged

Conversation

spacedmonkey
Copy link
Contributor

Context

Summary

Set the default value of tracks in trim component to an empty array. This brings this compontent inline with others like this, this and this.

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

  1. Create a story.
  2. Add video to canvas.
  3. Click trim video.
  4. See no error.

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #12471

@googleforcreators-bot
Copy link
Collaborator

googleforcreators-bot commented Oct 28, 2022

Plugin builds for e014eb1 are ready 🛎️!

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2022

Size Change: 0 B

Total Size: 2.72 MB

ℹ️ View Unchanged
Filename Size
assets/css/carousel-view-rtl.css 702 B
assets/css/carousel-view.css 701 B
assets/css/web-stories-block-rtl.css 4.5 kB
assets/css/web-stories-block.css 4.54 kB
assets/css/web-stories-embed-rtl.css 318 B
assets/css/web-stories-embed.css 317 B
assets/css/web-stories-list-styles-rtl.css 2.34 kB
assets/css/web-stories-list-styles.css 2.37 kB
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B
assets/css/web-stories-theme-style-twentyten.css 143 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 326 B
assets/css/web-stories-theme-style-twentytwentyone.css 326 B
assets/css/web-stories-widget-rtl.css 482 B
assets/css/web-stories-widget.css 482 B
assets/css/wp-dashboard-rtl.css 657 B
assets/css/wp-dashboard.css 659 B
assets/css/wp-story-editor-rtl.css 769 B
assets/css/wp-story-editor.css 771 B
assets/js/1629.js 193 kB
assets/js/2468.js 7.74 kB
assets/js/4422.js 49.3 kB
assets/js/6221.js 93.2 kB
assets/js/9162.js 36.5 kB
assets/js/9750.js 12.8 kB
assets/js/carousel-view.js 3.41 kB
assets/js/chunk-colorthief.js 2.64 kB
assets/js/chunk-ffmpeg.js 5.89 kB
assets/js/chunk-html-to-image.js 4.5 kB
assets/js/chunk-opentype.js 96 B
assets/js/chunk-react-calendar.js 12.5 kB
assets/js/chunk-react-color.js 44.3 kB
assets/js/chunk-selfie-segmentation.js 12.5 kB
assets/js/chunk-web-animations-js.js 14.6 kB
assets/js/chunk-web-stories-template-0-metaData.js 546 B
assets/js/chunk-web-stories-template-0.js 11.4 kB
assets/js/chunk-web-stories-template-1-metaData.js 540 B
assets/js/chunk-web-stories-template-1.js 9.6 kB
assets/js/chunk-web-stories-template-10-metaData.js 532 B
assets/js/chunk-web-stories-template-10.js 7.36 kB
assets/js/chunk-web-stories-template-11-metaData.js 539 B
assets/js/chunk-web-stories-template-11.js 9.07 kB
assets/js/chunk-web-stories-template-12-metaData.js 496 B
assets/js/chunk-web-stories-template-12.js 9.68 kB
assets/js/chunk-web-stories-template-13-metaData.js 525 B
assets/js/chunk-web-stories-template-13.js 7.39 kB
assets/js/chunk-web-stories-template-14-metaData.js 582 B
assets/js/chunk-web-stories-template-14.js 7.37 kB
assets/js/chunk-web-stories-template-15-metaData.js 544 B
assets/js/chunk-web-stories-template-15.js 9 kB
assets/js/chunk-web-stories-template-16-metaData.js 588 B
assets/js/chunk-web-stories-template-16.js 10.8 kB
assets/js/chunk-web-stories-template-17-metaData.js 540 B
assets/js/chunk-web-stories-template-17.js 9.2 kB
assets/js/chunk-web-stories-template-18-metaData.js 587 B
assets/js/chunk-web-stories-template-18.js 9.9 kB
assets/js/chunk-web-stories-template-19-metaData.js 501 B
assets/js/chunk-web-stories-template-19.js 10.8 kB
assets/js/chunk-web-stories-template-2-metaData.js 586 B
assets/js/chunk-web-stories-template-2.js 9.29 kB
assets/js/chunk-web-stories-template-20-metaData.js 548 B
assets/js/chunk-web-stories-template-20.js 8.99 kB
assets/js/chunk-web-stories-template-21-metaData.js 536 B
assets/js/chunk-web-stories-template-21.js 9.85 kB
assets/js/chunk-web-stories-template-22-metaData.js 525 B
assets/js/chunk-web-stories-template-22.js 7.83 kB
assets/js/chunk-web-stories-template-23-metaData.js 604 B
assets/js/chunk-web-stories-template-23.js 7.47 kB
assets/js/chunk-web-stories-template-24-metaData.js 517 B
assets/js/chunk-web-stories-template-24.js 11.7 kB
assets/js/chunk-web-stories-template-25-metaData.js 543 B
assets/js/chunk-web-stories-template-25.js 7.05 kB
assets/js/chunk-web-stories-template-26-metaData.js 600 B
assets/js/chunk-web-stories-template-26.js 7.26 kB
assets/js/chunk-web-stories-template-27-metaData.js 542 B
assets/js/chunk-web-stories-template-27.js 7.8 kB
assets/js/chunk-web-stories-template-28-metaData.js 532 B
assets/js/chunk-web-stories-template-28.js 9.05 kB
assets/js/chunk-web-stories-template-29-metaData.js 561 B
assets/js/chunk-web-stories-template-29.js 9.24 kB
assets/js/chunk-web-stories-template-3-metaData.js 539 B
assets/js/chunk-web-stories-template-3.js 8.39 kB
assets/js/chunk-web-stories-template-30-metaData.js 576 B
assets/js/chunk-web-stories-template-30.js 7.88 kB
assets/js/chunk-web-stories-template-31-metaData.js 503 B
assets/js/chunk-web-stories-template-31.js 10.3 kB
assets/js/chunk-web-stories-template-32-metaData.js 552 B
assets/js/chunk-web-stories-template-32.js 13.3 kB
assets/js/chunk-web-stories-template-33-metaData.js 491 B
assets/js/chunk-web-stories-template-33.js 9.06 kB
assets/js/chunk-web-stories-template-34-metaData.js 570 B
assets/js/chunk-web-stories-template-34.js 7.57 kB
assets/js/chunk-web-stories-template-35-metaData.js 565 B
assets/js/chunk-web-stories-template-35.js 8.9 kB
assets/js/chunk-web-stories-template-36-metaData.js 575 B
assets/js/chunk-web-stories-template-36.js 12.7 kB
assets/js/chunk-web-stories-template-37-metaData.js 529 B
assets/js/chunk-web-stories-template-37.js 6.71 kB
assets/js/chunk-web-stories-template-38-metaData.js 572 B
assets/js/chunk-web-stories-template-38.js 7.92 kB
assets/js/chunk-web-stories-template-39-metaData.js 589 B
assets/js/chunk-web-stories-template-39.js 8.07 kB
assets/js/chunk-web-stories-template-4-metaData.js 564 B
assets/js/chunk-web-stories-template-4.js 12.7 kB
assets/js/chunk-web-stories-template-40-metaData.js 556 B
assets/js/chunk-web-stories-template-40.js 10.2 kB
assets/js/chunk-web-stories-template-41-metaData.js 573 B
assets/js/chunk-web-stories-template-41.js 7.74 kB
assets/js/chunk-web-stories-template-42-metaData.js 521 B
assets/js/chunk-web-stories-template-42.js 6.99 kB
assets/js/chunk-web-stories-template-43-metaData.js 557 B
assets/js/chunk-web-stories-template-43.js 8.75 kB
assets/js/chunk-web-stories-template-44-metaData.js 583 B
assets/js/chunk-web-stories-template-44.js 11.1 kB
assets/js/chunk-web-stories-template-45-metaData.js 565 B
assets/js/chunk-web-stories-template-45.js 7.51 kB
assets/js/chunk-web-stories-template-46-metaData.js 531 B
assets/js/chunk-web-stories-template-46.js 5.21 kB
assets/js/chunk-web-stories-template-47-metaData.js 591 B
assets/js/chunk-web-stories-template-47.js 9.41 kB
assets/js/chunk-web-stories-template-48-metaData.js 556 B
assets/js/chunk-web-stories-template-48.js 9.08 kB
assets/js/chunk-web-stories-template-49-metaData.js 518 B
assets/js/chunk-web-stories-template-49.js 9.69 kB
assets/js/chunk-web-stories-template-5-metaData.js 556 B
assets/js/chunk-web-stories-template-5.js 9.91 kB
assets/js/chunk-web-stories-template-50-metaData.js 503 B
assets/js/chunk-web-stories-template-50.js 9.13 kB
assets/js/chunk-web-stories-template-51-metaData.js 526 B
assets/js/chunk-web-stories-template-51.js 10.3 kB
assets/js/chunk-web-stories-template-52-metaData.js 601 B
assets/js/chunk-web-stories-template-52.js 10.3 kB
assets/js/chunk-web-stories-template-53-metaData.js 551 B
assets/js/chunk-web-stories-template-53.js 5.78 kB
assets/js/chunk-web-stories-template-54-metaData.js 547 B
assets/js/chunk-web-stories-template-54.js 7.66 kB
assets/js/chunk-web-stories-template-55-metaData.js 574 B
assets/js/chunk-web-stories-template-55.js 7.12 kB
assets/js/chunk-web-stories-template-56-metaData.js 542 B
assets/js/chunk-web-stories-template-56.js 9.85 kB
assets/js/chunk-web-stories-template-57-metaData.js 528 B
assets/js/chunk-web-stories-template-57.js 14.9 kB
assets/js/chunk-web-stories-template-58-metaData.js 554 B
assets/js/chunk-web-stories-template-58.js 5.74 kB
assets/js/chunk-web-stories-template-59-metaData.js 590 B
assets/js/chunk-web-stories-template-59.js 8.94 kB
assets/js/chunk-web-stories-template-6-metaData.js 568 B
assets/js/chunk-web-stories-template-6.js 7.06 kB
assets/js/chunk-web-stories-template-60-metaData.js 509 B
assets/js/chunk-web-stories-template-60.js 9.52 kB
assets/js/chunk-web-stories-template-7-metaData.js 569 B
assets/js/chunk-web-stories-template-7.js 7.45 kB
assets/js/chunk-web-stories-template-8-metaData.js 569 B
assets/js/chunk-web-stories-template-8.js 8.9 kB
assets/js/chunk-web-stories-template-9-metaData.js 580 B
assets/js/chunk-web-stories-template-9.js 8.45 kB
assets/js/chunk-web-stories-templates.js 1.17 kB
assets/js/chunk-web-stories-textset-0.js 5.04 kB
assets/js/chunk-web-stories-textset-1.js 6.63 kB
assets/js/chunk-web-stories-textset-2.js 7.62 kB
assets/js/chunk-web-stories-textset-3.js 15 kB
assets/js/chunk-web-stories-textset-4.js 4.14 kB
assets/js/chunk-web-stories-textset-5.js 5.45 kB
assets/js/chunk-web-stories-textset-6.js 5.25 kB
assets/js/chunk-web-stories-textset-7.js 10.1 kB
assets/js/generateBlurhash.worker.worker.js 1.1 kB
assets/js/imgareaselect.js 3.77 kB
assets/js/lightbox.js 550 B
assets/js/tinymce-button.js 2.85 kB
assets/js/web-stories-activation-notice.js 27.1 kB
assets/js/web-stories-block.js 22.6 kB
assets/js/web-stories-embed.js 20 B
assets/js/web-stories-widget.js 587 B
assets/js/wp-dashboard.js 64.2 kB
assets/js/wp-story-editor.js 1.44 MB

compressed-size-action

@swissspidy
Copy link
Collaborator

swissspidy commented Oct 28, 2022

As per the ticket, we should add a data migration.

And of course ideally fix the root cause

@spacedmonkey
Copy link
Contributor Author

As per the ticket, we should add a data migration.

And of course ideally fix the root cause

This fix, fixes the problem. What is the point of adding a migration?

@swissspidy
Copy link
Collaborator

Looks like tracks is marked as optional on the VideoElement type, so I suppose this is expected.

In that case please fix the lint errors and then I think we can merge it.

@swissspidy swissspidy changed the title Video Trimming: Crash due to missing tracks property Video Trimming: Avoid crash due to missing tracks property Nov 1, 2022
@swissspidy swissspidy merged commit 80e2e2e into main Nov 3, 2022
@swissspidy swissspidy deleted the fix/12471 branch November 3, 2022 11:04
@kkalarickal
Copy link

@swissspidy - I was not able to find the branch within the web-stories-tester that would allow me to test this fix in isolation on stories-qa test site. To confirm this was fixed, I also recreated the crash in another branch that did not have the merged code.

However, I noticed that the #12471 issue was closed. So i tested this crash by trimming the video on 5th page of the specific story that I had mentioned in that ticket. And the crash did not occur.

Reporting this here since this ticket is sitting in Ready-for-QA although the fix seems to have been merged in main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Video Trimming: Crash due to missing tracks property
5 participants