Description
openedon Jul 22, 2018
Undo trap: The situation in which a user is "stuck" when continuously pressing Undo. The typical cause: as Undo is pressed and the content is reverted, some event somewhere causes a new undo level to be created. The result is that the user is unable to undo past a certain point, thereby losing access to previous editing states.
This issue aggregates all bugs stemming from that common problem:
- Gallery: Conversion from raw (Improve performance of the gallery block, by removing unnecessary requests. #6325, Fix undo trap in Gallery block when transforming from shortcode #42001)
- Embeds: Conversion from raw (Impossible to undo Convert to Blocks when post_content contains a core oEmbed link #7323)
- Image: Adding via upload (Image Block: Undo includes pulsing state as an undo step #3553)
- File: Adding via drag-and-drop (File block retains href after uploading an unsupported file type causing block to encounter an error after page refresh #8155)
Problem
At the root of these bugs is the fact that blocks need to manage a transition using specific ephemeral state:
- when converting from
[gallery ids="x,y,z"]
, a Gallery block needs to temporarily keep the IDsx, y, z
of the images while it waits for API data necessary to the block (the URL andalt
attribute for each image); once it receives the API data, it adds it to its attributes [resolved state]; - when adding an Image via file upload, the Image block initially keeps the image blob in order to render a pulsing image while waiting for upload conclusion; then, it updates its attributes to store the newly created URL for the uploaded image [resolved state];
Since the temporary states are encoded in the attributes themselves of the blocks, they automatically constitute an undo level of the editor history. Thus, the action of undoing will bring the blocks back to that temporary state, which itself triggers a transition "back" into the resolved state.
Solutions
There are, as I see it, two approaches:
- Altogether avoid encoding transitional states in a block's attributes. Since this is a problem that is likely to affect third-party blocks as well, ideally the problem wouldn't be fixed ad hoc but rather using a standardized method to express such states. It could be expressed in terms of creating a new block by passing it a "seed" for a future state (e.g. the seed of an image ID to later yield an image URL).
- Go in a very different direction and consider that certain "events" (term used loosely) in the application are made up of a sequence of action dispatches over time. So, for the constitution of a Gallery block converted from shortcode, the actions would be and ; these would fold into a single event, thus a single undo level. This approach is much more ambitious and likely to be overkill and less robust. I explain it at length in this comment: Improve performance of the gallery block, by removing unnecessary requests. #6325
The first option seems to be worth exploring first. A quick stab at the second one — by hacking withHistory
— doesn't inspire a lot of confidence.