Web-stories-wp: Story saved pointing to "blob" URL, causing video to fail loading in story

Created on 16 Jun 2020  路  18Comments  路  Source: google/web-stories-wp

Bug Description

For this story the background video in page one was saved using an incorrect blob url. Per @swissspidy 's review this is the markup:
<amp-video autoPlay="autoplay" poster="blob:https://stories-new-wordpress-amp.pantheonsite.io/13a6604b-2b51-4a6e-b4e6-7d0eceec4bf4" artwork="blob:https://stories-new-wordpress-amp.pantheonsite.io/13a6604b-2b51-4a6e-b4e6-7d0eceec4bf4" title="Bridge In Place" alt="Bridge In Place" layout="fill" id="el-5aff6cbd-9252-4e9d-a532-969cbde98f1a-media"><source type="video/mp4" src="blob:https://stories-new-wordpress-amp.pantheonsite.io/491491bd-b975-46b1-b337-5561e134bdce"/></amp-video>

Also from Pascal: "See the blob: thing. That means the video & images were probably still uploading at the time the story was saved, Of course a blob: URL isn鈥檛 really valid, hence the video fails to play"

Expected Behaviour

Story should be saved with the proper video URL, no blobs.

Steps to Reproduce

  1. Drag a video onto canvas to upload while creating story
  2. While video is uploading, save story/autosave?

Screenshots

Additional Context

  • Plugin Version:
  • Operating System:
  • Browser:

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance Criteria

Implementation Brief

Video Media Prometheus Bug Failed

All 18 comments

Here's another story where I can seem to repro (see page two here)

I repro'd by just creating a story and saving/previewing while I was working and dragging video to the canvas to upload and use in the story. Unsure of the specific steps that led to the broken video in page 2, but not in page 1, but I reproduced the issue quickly.

Since it seems so easy to reproduce, I suppose we can start asking what should ideally happen in this case.

First and foremost, I'd say saving and previewing should never ever result in blob: URLs in markup. We need to avoid that. But what should we output instead? Placeholders? Nothing?

Then, we might want to think about some UX changes to prevent this from happening in the first place.

For example, we could just disable all those actions (save/preview/publish) while upload is in progress.

Or, when you click on those buttons, they'll just wait for the upload to finish before doing anything.

For example, we could just disable all those actions (save/preview/publish) while upload is in progress.

Seems like this is already in place! But only for the publish button, not the preview button.

When clicking Preview we will:

// Save story directly if draft, otherwise, use auto-save.
const updateFunc = isDraft ? saveStory : autoSave;

that means the blob url will be persisted in the DB either way.

Should we disable the Preview button while something is uploading? (like Save/Publish buttons)
There are other solutions to this (e.g. save the story with blob url and update it after the upload is done) but require a lot more effort and can create more issues to solve.

Yeah, as a first measure for beta we should just disable the Preview button during uploads. At least in my opinion.

I like some of the other options above too, but we can follow-up on those post-beta.

@swissspidy @miina Disabling Preview (and maybe Save) until upload is done is a good idea. But we also need to disable auto-save, right?

Yep, good point 馃憤

Right, I will also disable autosave while uploading (other buttons are already disabled).

BTW: Autosave is relying on hasNewChanges which will be true if you just e.g. select any element (doesn't change story_data), this should be based on comparison between last story_data and current story_data, right?

BTW: Autosave is relying on hasNewChanges which will be true if you just e.g. select any element (doesn't change story_data), this should be based on comparison between last story_data and current story_data, right?

Yeah, that sounds like a bug then. Mind filing an issue?

@merapi hasNewChanges definitely doesn't seem to (or shouldn't) be true when just selecting a new element when I'm testing. Can you file an issue with steps to reproduce?

EDIT: E.g. the "Save draft" button is enabled only if hasNewChanges, however, it continues being disabled when just clicking through elements. Does it work differently for you?

@miina testing main branch, hasNewChanges is true after I select something.
https://drive.google.com/file/d/1ZePEpYtQvUGyF0UueZgaYG9izuiyciFM/view?usp=sharing

@miina testing main branch, hasNewChanges is true after I select something.

Hmm, okay, this seems to happen only in case of Media -- looks like selecting media is creating a new history entry for some reason (hasNewChanges is tracking new history entries compared to the saved version) -- e.g. you can see the Undo becoming active as well. Looks like there's something updating Media when selecting it even though nothing is actually changing, seems like a regression/bug indeed.

Edit: we should create a bug issue for "Selecting Media creates a new history entry" or sth similar.

Testing Instructions missing

@csossi see #2604 for instructions

Copying them here for visibility:

Try to save/preview the story (open Preview tab and check if the story is updated there) while something is uploading - you shouldn't be able to do it.

I'd like to add to this:

Observe the Network tab in dev tools and verify no autosave request is made while something is uploading

So basically:

Upload media -> verify that you cannot manually save the story while upload is in progress (i.e. disabled buttons)

Verified in QA

Button disabled when in mid-upload:
image.png

Verified in QA

Button disabled when in mid-upload:
image.png

Not sure why, but the Preview button is enabled on your screenshot (should be disabled because it saves the story when clicked). Maybe just a wrong branch on QA env?

I verified that it is actually disabled

Was this page helpful?
0 / 5 - 0 ratings

Related issues

swissspidy picture swissspidy  路  3Comments

LuckynaSan picture LuckynaSan  路  3Comments

injainja picture injainja  路  4Comments

injainja picture injainja  路  4Comments

3pgarro picture 3pgarro  路  4Comments