WordPress.org

Make WordPress Core

Opened 15 months ago

Last modified 5 weeks ago

#37887 new enhancement

Make attachments atomic until a Customizer session is published

Reported by: fjarrett Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: needs-patch
Focuses: administration Cc:

Description (last modified by westonruter)

Current behavior

When you upload attachments via a Customizer session they are:

  1. Added to the filesystem.
  2. Saved to the database as a new post.
  3. Immediately visible in the Media Library to all other logged in users.

Desired behavior

Attachments that are uploaded during a Customizer session should not be published, or even visible by other logged in users, until the user decides to publish the changes.

Possible idea

Until Customizer changed are published, all attachment posts that have been uploaded inside the Customizer could have their post status set to auto-draft rather than inherit. This will make them invisible inside the Media Library from other logged in users.

All attachment posts uploaded during the Customizer session could be stored inside the new Customizer transaction, and the Media Library query could be made to only show those uploads during the current unpublished session.

Publish or Discard

If a transaction (customizer changes) is published, then the post status on these attachment IDs can be set to inherit, making them visible inside in the Media Library like normal.

If a transaction (the customizer changes) is discarded, then the attachment IDs can be force deleted via wp_delete_attachment( $ID, true ) which will not only delete them from the database, but also from the filesystem.

Change History (16)

#1 @westonruter
15 months ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to Future Release

This is indeed an issue ever since the Media Library modal was introduced in the customizer. The Media Library JS is not Customizer-aware. Beyond uploading new attachments, there is also the issue whereby if you edit an attachment's fields (e.g. title or caption) then these changes get saved immediately as soon as you blur the field. All of these changes need to be added to the Customizer state to then be persisted if the user hits Save & Publish.

As for deleting attachments that are never published, what could done is to upload them with an auto-draft status (as opposed to inherit) and then only transition them to auto-draft once the customizer changes are saved. This would be taking the same approach as Customize Posts and the newly-committed #34923 which uses auto-draft posts as placeholders. Posts with auto-draft status are automatically garbage-collected after 7 days, though I'm not sure if an attachment post with an auto-draft status will get its associated uploaded file also deleted.

Custom fields in the media modal seem to be the most challenging to me. Plugins will have to take care to support the customizer usage. In particular, custom fields added to the media modal normally get saved by plugins via the attachment_fields_to_save action. Plugins would need to not do that in a customizer context, but instead add it to the customizer state and defer the save until the customizer state is published.

Queries for attachments in the media library would also need to include all existing attachment posts that have inherit status OR which are auto-draft and which have an ID that was created in the current customizer session. So that attachment SQL query would need to be updated to account for that.

All in all, I think it's doable, but it will take some serious R&D.

#2 follow-up: @celloexpressions
15 months ago

  • Keywords needs-patch added

This is also something that could be considered an issue with the media modal in the admin - within the context of any post, attachments probably shouldn't be published until the post is published.

That being said, I agree that leveraging the auto-draft status makes the most sense here, and actually may not be all that difficult to do. If all of the attachment meta is saved as it is now, that should still work because the associated attachment is still an auto-draft and therefore wouldn't be visible, right? For existing attachments, though, that would be an issue. We might want to focus first on new attachments and then pursue previewing changes to existing attachments, as that starts to get into things like previewing edits to actual image files where duplicating existing attachments as auto-drafts may become necessary.

This ticket was mentioned in Slack in #core-customize by helen. View the logs.


12 months ago

This ticket was mentioned in Slack in #core-media by westonruter. View the logs.


10 months ago

#5 follow-up: @azaozz
10 months ago

Desired behavior

Attachments that are uploaded during a Customizer session should not be published, or even visible by other logged in users, until the user decides to publish the changes.

Why?

Is there any advantage in hiding uploaded files from other trusted users? I don't see any. Similar case is when a file is uploaded while the user composes a draft. We don't hide the upload until the post is published.

I'm also not sure if it is a good idea to automatically delete a file that was uploaded while the customizer was used. This changes the current file uploading behavior for no good reason: the users are still seeing the (familiar) media popup, but files they upload may disappear later. This can be taken as error and is pretty bad UX.

Also, if hiding of uploads is really really needed by somebody, it should be a plugin. Seems it should be implemented by adding these attachments to a "special" taxonomy (or similar) and excluding them from the Media library. The proposed solution is needlessly complex.

Last edited 10 months ago by azaozz (previous) (diff)

#6 in reply to: ↑ 5 ; follow-up: @westonruter
10 months ago

Replying to azaozz:

Is there any advantage in hiding uploaded files from other trusted users? I don't see any. Similar case is when a file is uploaded while the user composes a draft. We don't hide the upload until the post is published.

Yes, though it's not about trust. If the files were uploaded as part of a live preview session, one which may be thrown away without ever wanting them to go live, then the uploaded file shouldn't appear when outside of that customization session. Consider a user who wants to see how a new radical design to their company logo appears on the site. They go into the customizer, go to change the custom logo and upload the new one. They then see how it appears on the site and decide it looks horrible and they want to abandon the changes. Currently in the customizer, this bad uploaded image would persist in the media library. But I think, as this ticket proposes, that the bad logo image should be eliminated when customized changes are abandoned.

I'm also not sure if it is a good idea to remove or delete a file that was uploaded while the customizer was used. This changes the current file uploading behavior for no good reason: the users are still seeing the (familiar) media popup, but files they upload may disappear later. This can be taken as error and is pretty bad UX.

Everything done in the customizer should be understood to be done in a preview context. Users should expect that nothing goes live until they hit Save & Publish. I think that users wouldn't necessarily expect that uploading an image via the media modal inside the customizer should result in that upload persisting. Also consider media attributes, like titles and descriptions: currently the media modal will write changes to these fields back to the database immediately. However, a user may want to preview how an attachment title change appears on the site and when in the customizer (live preview session) they should expect that any change made would not go live until they hit Save & Publish.

Also, if hiding of uploads is really really needed by somebody, it should be a plugin. Seems it should be implemented by adding these attachments to a "special" taxonomy (or similar) and excluding them from the Media library. The proposed solution is needlessly complex.

So it's not just hiding uploads that's in scope here. It's that any change made (including uploads) via the media modal should be encapsulated in a customization session (customize changeset). A first step for implementing this was done for theme starter content in fact for 4.7 (via #38615). When starter content is loaded into a customization session, none of the changes go live until the user hits Save & Publish, and this includes any images uploaded to the media library for use as featured images for posts/pages in the starter content. The starter content posts/pages/attachments are inserted with auto-draft status so that they will be garbage-collected if the user never publishes the changes. When they do publish the changes, then this is when the status is set to publish (or inherit).

I think think this should be part of core because it's bringing live preview to another area of WordPress and further minimizing the poor “save & surprise” user experience.

#7 follow-up: @fjarrett
10 months ago

Hey @azaozz, thanks for chiming in on this!

Is there any advantage in hiding uploaded files from other trusted users? I don't see any.

But the attachment is actually visible to the world, not just trusted logged-in users.

  1. Open the Customizer and upload a new header image
  2. Do not click Save & Publish - just exit the Customizer
  3. Go to the Media Library and click the image to open the Attachment Details modal
  4. Click "View attachment page" - this is a public URL

Making images public to the world without clicking Save & Publish is definitely an unexpected UX.

The current behavior of attachments added during a Customizer session make the Save & Publish button a partial truth.

#8 in reply to: ↑ 2 @SergeyBiryukov
10 months ago

Replying to celloexpressions:

This is also something that could be considered an issue with the media modal in the admin - within the context of any post, attachments probably shouldn't be published until the post is published.

Related: #17255

#9 in reply to: ↑ 6 ; follow-up: @azaozz
10 months ago

Replying to westonruter:

Yes, though it's not about trust. If the files were uploaded as part of a live preview session, one which may be thrown away without ever wanting them to go live, then the uploaded file shouldn't appear when outside of that customization session. Consider a user who wants to see how a new radical design to their company logo appears on the site. They go into the customizer, go to change the custom logo and upload the new one. They then see how it appears on the site and decide it looks horrible and they want to abandon the changes. Currently in the customizer, this bad uploaded image would persist in the media library. But I think, as this ticket proposes, that the bad logo image should be eliminated when customized changes are abandoned.

Still don't think auto-deleting the image is right. It is very easy for the user to delete it if they want, or to keep it for later. There is even the user case where the user may want to let somebody else see what it looks like before setting it as logo.

Also, what if an existing file is used in the same session? Don't think we would want to delete it. Why should the behavior be any different?

There is a well established workflow when uploading files. Changing it just because a file is uploaded from another place is not good UX imho. Having "auto-disappearing" images in the media popup/library is bad UX.

I agree that there are user cases when auto-deleting/removing an upload makes sense, logically. But there are cases when it doesn't. The more important (UX) part of it is that the users will not expect uploads to disappear from the media library on their own.

Everything done in the customizer should be understood to be done in a preview context. Users should expect that nothing goes live until they hit Save & Publish.

Yeah, I understand. However imho this is one of the weak points of the customizer UI: there is nothing to suggest this behavior. Even the opposite, new users often assume that their site *is* the preview and all is already "live".

I think that users wouldn't necessarily expect that uploading an image via the media modal inside the customizer should result in that upload persisting.

Don't think so. There is nothing to suggest to the users that the (familiar) media modal will work differently. Changing the way it works at different places in wp-admin is pretty tough. Not telling the users that it works differently is unacceptable.

So it's not just hiding uploads that's in scope here. It's that any change made (including uploads) via the media modal should be encapsulated in a customization session (customize changeset).

Not sure about that either. What will happen if we add inline writing/editing of posts in the customizer "preview"? Are we going to change the current publishing workflow just because the post is being edited from the customizer iframe? Are we going to delete the post if the user doesn't click "Save & Publish"? What about drafts and autosave? How are we going to explain that to the users? And most importantly: why?

I think think this should be part of core because it's bringing live preview to another area of WordPress and further minimizing the poor “save & surprise” user experience.

Don't think changing how the media modal works, and/or auto-deleting attachments has anything to do with "live" or "non-live" previews. I think it will bring even more unexpected/bad UX to the customizer.

#10 in reply to: ↑ 9 ; follow-up: @westonruter
10 months ago

Replying to azaozz:

Still don't think auto-deleting the image is right. It is very easy for the user to delete it if they want, or to keep it for later. There is even the user case where the user may want to let somebody else see what it looks like before setting it as logo.

The image would only be auto-deleted if there is no reference to the attachment. Since it's suggested here that uploads and attachment changes should be encapsulated inside of a customization session (changeset), if that changeset is abandoned/deleted then the attachment should also be purged. As of 4.7, a user can share the URL to a customizer session (with the changeset_uuid param intact) with another user so that they can see the image applied as the site logo. A user can even share a URL to the frontend directly with the customize_changeset_uuid param present and let an unauthenticated user see how the custom logo appears.

Everything done in the customizer should be understood to be done in a preview context. Users should expect that nothing goes live until they hit Save & Publish.

Yeah, I understand. However imho this is one of the weak points of the customizer UI: there is nothing to suggest this behavior. Even the opposite, new users often assume that their site *is* the preview and all is already "live".

There is a help (?) icon in the root customizer panel that includes text saying, “The Customizer allows you to preview changes to your site before publishing them.” If it is not clear to users that the changes made in the customizer are non-destructive (and it surely can be made more clear that all changes made are pending), this means we need to make this more clear, not be OK with some things being destructive (or rather constructive) prior to doing Save & Publish.

Not sure about that either. What will happen if we add inline writing/editing of posts in the customizer "preview"? Are we going to change the current publishing workflow just because the post is being edited from the customizer iframe? Are we going to delete the post if the user doesn't click "Save & Publish"? What about drafts and autosave? How are we going to explain that to the users? And most importantly: why?

Yes. My position is that anything done during customization w/ live preview should be non-destructive (non-mutating) until the user explicitly wants the changes to go live. This needn't be restricted to the customize preview. In the future if we allow entering into a customization session from the frontend (i.e. frontend editing) the same principle should apply. Everything done to the site should be “staged” in a draft (changeset) that goes live once the user is happy with the changes. A missing piece in the current customizer implementation is that there is no UI for saving a draft of changes: the customize_changeset posts created during customizer sessions are always created with the auto-draft status (as well as the posts created for any page/post stubs). Allowing changesets to be longer-lived drafts is what #31089 and the Customize Snapshots feature plugin are addressing.

The customizer certainly needs UX improvements and its UI needs an overhaul, and this will be a focus for 2017. But I hold that the more aspects of a site to which we can add preview support the better—whether this be options, widgets, nav menus, posts/pages, taxonomy terms, media, and so on. If media preview support is added as proposed in this ticket, then there would certainly need to be UX changes to the media modal.

Last edited 10 months ago by westonruter (previous) (diff)

#11 in reply to: ↑ 7 @azaozz
10 months ago

Replying to fjarrett:

But the attachment is actually visible to the world, not just trusted logged-in users.

  1. Open the Customizer and upload a new header image
  2. Do not click Save & Publish - just exit the Customizer
  3. Go to the Media Library and click the image to open the Attachment Details modal
  4. Click "View attachment page" - this is a public URL

Another case:

  1. Open the Customizer and upload a new header image
  2. Do not click Save & Publish - just exit the Customizer
  3. Talk to your colleague that is also an admin on the site and ask them to see if the image you uploaded for header background is good.
  4. What image?

Yet another case:

  1. Open the Customizer and select an existing header image
  2. Do not click Save & Publish - just exit the Customizer
  3. Go to the Media Library. Would you expect the image you selected for header background to still be there?

Making images public to the world without clicking Save & Publish is definitely an unexpected UX.

No, its not. This is how uploading works in WordPress and is the simplest, most logical way. Look at uploading images on draft posts. If this is ever changed for posts, we can use the same workflow in the customizer, but I don't think the workflows should be different.

Also auto-deleting uploaded files in some specific cases will always be bad UX. We will be guessing what the user intent may be and will definitely get it wrong sometimes. (And, well, we will be deleting files which is a destructive action. What if the user doesn't realize the file was deleted and deletes the original from their computer. Then after a few days... My file is missing!!! WordPress ate my homework!!'', etc.).

There is also the fact that uploaded files cannot ever be 100% private as the wp-content/uploads directory is publicly accessible. This is the main reason there is no "trashed" state for attachments. To make this 100% the files have to be moved above the web server root.

Last edited 10 months ago by azaozz (previous) (diff)

#12 in reply to: ↑ 10 @azaozz
10 months ago

Replying to westonruter:

My position is that anything done during customization w/ live preview should be non-destructive (non-mutating) until the user explicitly wants the changes to go live. This needn't be restricted to the customize preview. In the future if we allow entering into a customization session from the frontend (i.e. frontend editing) the same principle should apply. Everything done to the site should be “staged” in a draft (changeset) that goes live once the user is happy with the changes.

Yeah, that makes sense for the way the customizer currently works: provide UI for changing the settings that can be previewed, and show a preview (in another area on the screen).

However this workflow is no good in some more complex cases. For example: writing and editing posts in the customizer iframe. There you have a "draft" state that is independent of other settings or "components" of the same post, like media that is embedded in it. So what would happen to images, audio, video uploaded to the draft post?

A missing piece in the current customizer implementation is that there is no UI for saving a draft of changes: the customize_changeset posts created during customizer sessions are always created with the auto-draft status (as well as the posts created for any page/post stubs). Allowing changesets to be longer-lived drafts is what #31089 and the Customize Snapshots feature plugin are addressing.

Same as above. If that gets implemented, and the user saves a "draft of changes", what will happen to images uploaded while in the customizer, and images selected for some purpose while in the customizer, and why these cases should be different?

#13 follow-up: @westonruter
10 months ago

@azaozz good questions! We'll have to figure out answers for them this year as we work toward creating a unified site building experience that converges editing with customization.

#14 in reply to: ↑ 13 @fjarrett
10 months ago

Yes, thanks @azaozz for weighing in with a different perspective on this.

@westonruter - Just to toss in another idea while we're getting it all out there :-)

When admins are browsing the filesystem, I think it should be clearly indicated which attachments belong to Customizer changesets, and should therefore be considered temporary.

To help make this distinction between normal and pending attachments, the file names could contain the changeset UUID in a prefix, and be saved as hidden dot-files. This would apply to every attachment size variation as well:

uploads/2017/01/.customize_changeset_3c5fee75-2745-4b14-b059-e947aad2964b_dog.jpg
uploads/2017/01/.customize_changeset_3c5fee75-2745-4b14-b059-e947aad2964b_dog-150x150.jpg
uploads/2017/01/cat.jpg
uploads/2017/01/cat-150x150.jpg

And once the changeset is published:

uploads/2017/01/cat.jpg
uploads/2017/01/cat-150x150.jpg
uploads/2017/01/dog.jpg
uploads/2017/01/dog-150x150.jpg

This is also one of those rare situations where it would actually be OK to modify the posts.guid since the attachments have never been live.

Last edited 10 months ago by fjarrett (previous) (diff)

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


7 months ago

#16 @westonruter
5 weeks ago

In 41872:

Widgets: Update preview for Gallery widget when one of its attachments is modified in the media modal, outside the customized state.

  • Ensure that changes to captions are shown in preview when modified in media modal.
  • Also keep wp.customize.widgetsPreview.renderedWidgets updated when widgets are added or removed.

See #41914, #37887, #40403.
Fixes #41979.

Note: See TracTickets for help on using tickets.