WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#23220 closed task (blessed) (fixed)

Extend autosave to use the browser's local storage in addition to saving to the server

Reported by: azaozz Owned by:
Milestone: 3.6 Priority: normal
Severity: normal Version:
Component: Autosave Keywords: autosave-redo
Focuses: Cc:

Description

There are several types of local (DOM) storage in the modern browsers. Most suitable is the localStorage as it's persistent and supported in all browsers back to IE8.

The idea is to save the content in local storage every 10 seconds and push to the server every two minutes. When the server responds and there are no errors, we empty the local storage and start again. The local storage will also be emptied when the user saves the post. Then every time the user visits the admin we can check if the local storage is not empty and offer to recover from there or show a revision diff.

Attachments (9)

AutoSave.patch (4.4 KB) - added by asannad 5 years ago.
AutoSave-v2.patch (7.8 KB) - added by asannad 5 years ago.
Atosave-v3.patch (10.8 KB) - added by asannad 5 years ago.
23220-4.patch (19.0 KB) - added by azaozz 5 years ago.
23220-5.patch (20.3 KB) - added by azaozz 5 years ago.
23220-6.patch (18.9 KB) - added by azaozz 5 years ago.
disable_save_while_disconnected.diff (2.3 KB) - added by dh-shredder 5 years ago.
Disables Publish and Save Draft buttons during autosave and while disconnected.
disable_save_while_disconnected.2.diff (3.6 KB) - added by dh-shredder 5 years ago.
Only disabling buttons during after autosave if wp.heartbeat.connectionLost
disable_save_while_disconnected.3.diff (3.1 KB) - added by dh-shredder 5 years ago.
Simplified code per @azaozz's suggestions.

Download all attachments as: .zip

Change History (38)

#1 @azaozz
5 years ago

The localStorage as implemented in the browsers has some limitations: it can store only string key => value pairs. Commonly the values are JSON encoded. Also each "site" has one localStorage, meaning we need to namespace the keys or perhaps store more complex JSON objects.

#2 @rmccue
5 years ago

  • Keywords autosave-redo added

#3 follow-up: @azaozz
5 years ago

Replying to F J Kaiser:

I just read about this on the Mozilla site - article from May 2012. Aside from the statement "terrible performance", I wonder which browsers support this. WP has always cared about a lot of browsers and I doubt that IE7 has a proper equivalent of localStorage.

Yes, this article describes the current state of browser storage quite well. The "terrible performance" of localStorage is mostly when storing images or other binary files, however we will be storing text only so don't think this is a concern.

That's right, IE7 doesn't have localStorage or any other type of browser storage. It is very near its EOL, we may stop supporting it even in 3.6 depending on the latest stats (see #18199).

#4 follow-up: @wonderboymusic
5 years ago

I use this:
https://github.com/marcuswestin/store.js

Here's the version with proper semicolons - the worst JS syntax is written by Python hackers:
https://github.com/StevenBlack/store.js

#5 in reply to: ↑ 3 ; follow-up: @F J Kaiser
5 years ago

  • Cc 24-7@… added

As a reference, here's the list of browsers supporting localStorage (no idea how much support is there for sessionStorage). Doesn't look too bad, but it (the ticket) will have to get punted after having IE7 deprecated in #18199.

Replying to azaozz:

Yes, this article describes the current state of browser storage quite well. The "terrible performance" of localStorage is mostly when storing images or other binary files, however we will be storing text only so don't think this is a concern.

That wouldn't be the only concern we should have. You can live test your browser here to see the limit of the localStorage and how it differs between browsers. I guess this will be one of the main things where devs will hit an invisible wall that's hard to debug as long you don't know what exactly hit you.

We know how the majority of wanna-be devs works. And cleaning things up, checking for errors and limits (memory) isn't an idea that's too wide spread. So it's not only about pushing files or images around, but also about moving really big JSON encoded strings like multiple page spanning tables (like in MS Excel) around. To make an example of what people use WP for, read this example, I answered a year ago. We have to consider the fact that lots of people use WP as base for really heavy tool Web APPs nowadays.

What I like about this idea is the tactical approach behind it, already written in the title: "extend autosave (...) in addition to saving to the server". This will allow us to educate devs a little, before throwing this with the "too good to be true"-Heartbeat API (see #23216) at them. Is this enough? I don't know. The best would be to have it _deadsimple_ and 3-function (get/set/dump) API wrapped around.

#6 @alex-ye
5 years ago

  • Cc nashwan.doaqan@… added

#7 in reply to: ↑ 4 @azaozz
5 years ago

Replying to wonderboymusic:

I use this:
https://github.com/marcuswestin/store.js

Yes, localStorage is really simple to work with. This looks a bit outdated, for example globalStorage has been obsolete since Gecko 1.9.1 (Firefox 3.5) and unsupported since Gecko 13 (Firefox 13): https://developer.mozilla.org/en-US/docs/DOM/Storage#Compatibility_and_relation_with_globalStorage

Also not sure we need to simulate localStorage if it's not supported (IE7, Opera mini). The main reason we will be using it is to save the content to the user's hard disk.

#8 in reply to: ↑ 5 @azaozz
5 years ago

Replying to F J Kaiser:

That wouldn't be the only concern we should have...

Yes, this is one of the weaknesses of the current implementation of localStorage in the browsers: there is no way to check how much storage is available. On top of that there is no easy way (Firefox) or no way at all (WebKit, IE) for the user to allocate more space. Only Opera pops up a dialog when the storage is full and lets the user make it "unlimited".

The way around this would be to remove a revision when adding new revision fails. For example, if we decide to store 5 revisions, saving another revision would remove the oldest before attempting to add the newest. If adding the newest fails, we can try removing the next oldest to free some space, etc.

... The best would be to have it _deadsimple_ and 3-function (get/set/dump) API wrapped around.

I'm not even sure we need an API, localStorage is super simple to work with. The only thing an API could add would be to do JSON encoding and decoding of the values (see the script linked in @wonderboymusic's comment).

@asannad
5 years ago

#9 @asannad
5 years ago

AutoSave.patch​ contains initial patch.
couple of things pending

  1. Show user that diff is available when they login.
  2. Show diff and then option to restore.

#10 @adamsilverstein
5 years ago

  • Cc adamsilverstein@… added

@asannad
5 years ago

#12 @asannad
5 years ago

I made some progress on this. AutoSave-v2.patch has those. Still I have to work on showing the recover dialog on admin screen. For that I am not able to decide how to load my js on all admin pages.

Last edited 5 years ago by asannad (previous) (diff)

@asannad
5 years ago

#13 @asannad
5 years ago

Modified atosave-v3.patch attached.

  1. Changed data structure to save data as

localstorage['wp_autosave_'.blogi_id => array( 'post_id' => array(
1=> data1,
2=> data2,
till count 5)

);

  1. Not deleting data in either autosave or post update. Will check for post data save time on page load and remove it if its stale for more than 12 hours.
  1. Post recover dialog on admin pages is done now
Last edited 5 years ago by asannad (previous) (diff)

@azaozz
5 years ago

#14 @azaozz
5 years ago

23220-5.patch improves on Atosave-v3.patch and adds methods to preview and restore the local backups when on the Add/Edit Post screen. Also adds translated strings and styling.

This is ready for broader testing and could need some UI tweaks.

@azaozz
5 years ago

#15 @azaozz
5 years ago

The above patch creates a "local backup system" that stores the content on the user's hard disk for 24 hours and has a simple UI to restore that content if the user ever needs it. As discussed in IRC, this may be more than we need. Another concern when using localStorage is that anything saved there will stay on the computer's HD until deleted which is not ideal for public or multi-user computers where all users share the same account.

In that terms we should go in another direction. The localStorage should only be used between autosaves and while saving the post and emptied right after.

The main reasons for loosing content are:

  1. Bad connectivity. The Internet connection is down or is very slow/has high packet loss resulting in the server timing out.
  2. Expired login cookies or nonces. This would be handled better when #23295 gets in.
  3. JS fatal errors. Most errors occur at page load, but we use more and more XHRs that may cause a fatal error and prevent proper saving or send empty post content. Relevant when the Visual editor is used.
  4. Browser or computer crash.
  5. Partial or corrupted content (extremely rare).

Cases (1), (2) and (4) can be avoided by storing the last autosave data in browserStorage and re-send it to the server after the connection is restored or the user has logged in again. In this case all can be handled in the background.

For case (5) we can add a simple integrity check (strlen() should be enough) on the post content, verify it in PHP and set a 'success' flag on the redirect (next page load). The same flag would be used to delete the data from localStorage.

Case (3) would be harder to catch. Depending on where and when the JS error occurs TinyMCE may still work properly. In some cases other functions can fail and the textarea can be empty despite that the content is still in the editor. So if $_POST['content'] is empty we should be rejecting the save/autosave and perhaps asking the user to reload.

#16 @knutsp
5 years ago

  • Cc knut@… added

I like this approach.

if $_POST[´content´] is empty we should be rejecting the save/autosave and perhaps asking the user to reload

This may very well be intended. But if previously saved content is long and then suddenly is empty, it may be because of such error.

Could an [empty] (core) shortcode, that expands to an empty string, be a way to indicate that the content is intentionally left empty?

#17 @azaozz
5 years ago

This may very well be intended. But if previously saved content is long and then suddenly is empty, it may be because of such error.

Yes, for CPTs this may be intended. Perhaps we can compare the new save/autosave 'content' with the previous autosave, or check this only for specific post types. Of course saves for CPTs that don't support content would skip that test.

#18 @Viper007Bond
5 years ago

Note that localStorage space limitations in Firefox are for all subdomains of a domain, per the spec. This prevents abuses like the one described here: http://feross.org/fill-disk/

This means for example that all WordPress.com blogs as a whole would be limited to 5 megabytes, not per blog (in the admin area where we don't use domain mapping).

With this in mind, we should make sure to store as little data as possible otherwise people who blog on lots of different sites will quickly run out of space and have the functionality no longer work for them.

@azaozz
5 years ago

#19 @azaozz
5 years ago

In 23220-6.patch​:

  • Use sessionStorage. Advantages: better security - auto-removed when the browser window/tab is closed. No size restrictions in most browsers. Disadvantages: doesn't persist after the browser tab is closed. Session storage covers the most common cause of lost content: errors on submitting the form caused by lost connection, nonces, browser crashes, etc. The only limitation is that the user has to stay in the current browser tab and click the Back button.
  • Minimal UI letting the user know the current post doesn't match the stored backup. The content can be restored and then the restore can be undone.
  • Empty the storage on logging out.

The main advantage of using localStorage is that the user can quit the browser, open it later, log in, and the unsaved changes can be auto-saved. This would benefit mostly users that have very unreliable Internet connection, perhaps travelling, etc. A big disadvantage is that the 5MB size is shared for subdomains as @Viper007Bond points out above.

The best way to implement localStorage is to ask the user to enable it on each computer he logs on from and to have control over emptying the storage. However this moves into a plugin territory. It is possible to switch between sessionStorage and localStorage depending on the login terms (the Remember Me checkbox). However the differences will be quite hard to explain to most users and confusing for probably all users.

#20 @azaozz
5 years ago

In 23683:

Autosave to the browser's sessionStorage, compare this autosave to the post content on page load and let the user restore it when the data is not the same. First run, see #23220

#21 @azaozz
5 years ago

In 23693:

Local autosave: set a temp cookie on submitting the form and change it on redirecting after the post is saved/updated, then use it to determine if saving worked properly. Removes the chance for false positives after saving/updating a post. See #23220

@dh-shredder
5 years ago

Disables Publish and Save Draft buttons during autosave and while disconnected.

#22 @azaozz
5 years ago

In 23705:

Local autosave: remove the temp cookie after first use. Prevents false negative if the browser crashes. Add some temp logging to aid testing. See #23220

@dh-shredder
5 years ago

Only disabling buttons during after autosave if wp.heartbeat.connectionLost

#23 @dh-shredder
5 years ago

Attached disable_save_while_disconnected.2.diff, which disables buttons when heartbeat connection is lost, and displays error message. Error is removed when connection is restored.

Likely the trigger-checks should go in a different file (heartbeat?), and have screen detection instead, but posting as-is for testing and discussion. Because we're not disabling after first failed autosave, there's much more latency before the user knows that the connection is lost.

Should we mark connection as lost after *either* a number of failed heartbeats or time range instead of just time range? Thinking that if we have >3 failed heartbeat attempts in a row, that's a good sign we have connection issues.

#24 @azaozz
5 years ago

In 23796:

Local autosave: remove the locally stored data after a post is saved. This prevents false positives when several users edit the same post multiple times. See #23220

@dh-shredder
5 years ago

Simplified code per @azaozz's suggestions.

#25 @azaozz
5 years ago

In 23886:

Autosave: use heartbeat to determine when connection is lost and disable the Save and Publish buttons. Re-enable the buttons when connection is restored. Props dh-shredder, see #23220

#26 @azaozz
5 years ago

In 24221:

Local autosaves: show the notice above the post formats UI, see #23220

#27 @azaozz
5 years ago

In [24269]:

Local autosaves: remove debug logging, see #23220

#28 @azaozz
5 years ago

In 24431:

Autosave: properly set autosaveLast when TinyMCE is the default editor. Prevents firing autosave when there are no changes. See #23220

#29 @azaozz
5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Works as expected. New tickets for any bugs.

Note: See TracTickets for help on using tickets.