The Problem
If you start entering information into a Drupal form, accidentally click a link, then use the back button to go back to the form, ckeditor4 input is lost.
Proposed solution
Remaining tasks
The behaviour needs to be tested on more browsers, and on ckeditor5.
Original report
Here is an easy way to lose data, and its extremely frustrating and is reproduced frequently among my users and occasionally by myself.
Create a new page, type a paragraph, hit preview. Type several more paragraphs, now click a link or hit (in firefox) alt+home. (this is normally done on accident by the user, but can also be done by hitting the "formatting tips" link if they want help)
The obvious consequence is that the user leaves the page. They hit back, to see their work, but because they hit the preview button earlier, when they hit back, drupal has to resubmit the form (as it was before adding the additional paragraphs) causing them to lose all the work they recently did.
Note that under normal circumstances, if the preview button is not used, this data loss doesn't occur. They hit the back button and everything is fine.
Possible solutions:
* confirm(nag) - warn them that they have unsaved changes, and are they sure they want to leave the page? (blogger, gmail and many other progs do this) - still a hack, but is better than losing data.
* cookie - every 10th key press or something, save the contents of the form to a cook - yuck, but better than losing data.
* save data persistently, either through an automated save draft (gmail does this) and/or a "save and continue editing" button (wordpress does this) - ideal
* redirect after post - that way user's can still use their back button in the normal way - ideal
Comment | File | Size | Author |
---|---|---|---|
#118 | form_autosave_garlic-153313-118.patch | 33.28 KB | Wim Leers |
#118 | interdiff.txt | 2.77 KB | Wim Leers |
#112 | form_autosave_garlic-153313-112.patch | 31.21 KB | Wim Leers |
#112 | interdiff.txt | 16.76 KB | Wim Leers |
#107 | drupal-form-data-back-153313-107.patch | 29.92 KB | Wim Leers |
Comments
Comment #1
cosmicdreams CreditAttribution: cosmicdreams commentedI like the save and continue editing button idea. I'm forwarding this post to the user experience group.
Comment #2
Sutharsan CreditAttribution: Sutharsan commentedMoving issues from User experience project to Drupal core usability component.
Comment #3
David_Rothstein CreditAttribution: David_Rothstein commentedI tried reproducing this in Drupal 5, 6, and 7 (using Firefox 3 on Linux). In my experience, the preview button had nothing to do with it -- I was able to experience the problem both with and without hitting the preview button. Here's what I found:
This suggests that it could be a bug in the javascript that was introduced in Drupal 6. Although the behavior could very well be different in different browsers.
Comment #4
Carl Johan CreditAttribution: Carl Johan commentedSome browsers will save your halfway-filled-out form for you if you navigate away from it, and then fill it in when you navigate back to the form page.
I'm not sure what the circumstances are to trigger this behaviour in browsers, but I'd say it's more worthwhile to try and create a browser (and form?) independent way of doing this.
Comment #5
David_Rothstein CreditAttribution: David_Rothstein commentedNote related issue: #655388: Many ways to lose data on form input in the overlay
(It's actually the same issue in many ways, but the overlay is a special case so it might have some problems along these lines that are unique to it. The data loss bug occurs with or without the overlay, though.)
Comment #6
Alex UA CreditAttribution: Alex UA commentedSubscribing- I've definitely lost a few hours of work due to this bug.
Comment #7
brianV CreditAttribution: brianV commentedPerhaps we need to implement a system where me make better use of the 'Unpublished' state.
1. As long as the post has a title, save it as 'Unpublished' every X seconds.
2. Before previewing, save it as well.
3. Don't publish until user hits 'Publish' button.
If the user is updating, do the above three steps, but save into a new unpublished revision, which becomes published in turn. Perhaps 'Publish' button is changed to 'Publish changes'.
Comment #8
tumblingmug CreditAttribution: tumblingmug commentedAs David_Rothstein said: the back button functionality got lost if JS is enabled; so it should be possible to catch the data loss by an JS alert, that warns about consequences leaving the page and offers a cancel button. So both cases (with and without JS) could provide a relative sure editing then. The way brianV suggests, lets things appear very close to WordPress on one hand; on the other hand this would cost performance for sure - if you think of an active community site with many people editing at the same time. WordPress' usabilty has to pay for it and it does. But the goals are quite different, even if usability is an issue of both systems.
Comment #9
chx CreditAttribution: chx commentedThis is not critical. Fix your browser first to support http://ejohn.org/blog/dom-storage/ this. Second, nothing ever stopped writing a contrib mdoule that throws up an alert onbeforeunload (and then Opera does not support that but then Opera has excellent back).
Comment #10
Alex UA CreditAttribution: Alex UA commentedchx- your comment is not really relevant, and telling users to update to browser X, especially when we know that Drupal's JS is responsible for the problem isn't going to fly when that user loses an hour of work.
Moving back to critical, please ignore this chx if you don't like the issue, but please respect that people other than you regard this as critical.
Comment #11
xmacinfoI have to agree that this is not a critical issue. :-)
However I agree that we need a fix for this. A "Dirty form" JavaScript function was added to the overlay module and removed later -> #517688: Initial D7UX admin overlay. I guess the warning fired up way to frequently to users taste.
We should have a tone down version which would fire up only for various content type creation form.
If we want to auto save data, we must also provide a way to delete these "draft" content, but that would make it only in D8.
Comment #12
Dries CreditAttribution: Dries commentedI agree that this is very annoying, but at the end of the day, it is not critical. Critical bugs stop Drupal from working.
Comment #13
mikeker CreditAttribution: mikeker commentedIn case people stumble onto this thread looking for a D6 solution...
The advantage of the "nag" option is that no data will be lost (eg: Draft only auto-saves every 30 seconds by default and there are bandwidth issues if you make it something like 3 seconds). But Draft gives you a "Save as Draft" button which is easier to explain to clients than "first click on Publishing options and then uncheck Published."
No reason you can't have both "nag" and auto-save options running at the same time, I suppose...
- Mike
Comment #14
ksenzeeSubscribing. If I get the time, I'll investigate how exactly the node form Javascript could be breaking the back button. Something odd is going on there.
Comment #15
casey CreditAttribution: casey commented#239132: Keep users from unintentionally navigating away from node preview pages.
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedSubscribe
Comment #17
David_Rothstein CreditAttribution: David_Rothstein commentedI can't reproduce this anymore in Drupal 7 (except when the Overlay is enabled, but that has its own issue at #655388: Many ways to lose data on form input in the overlay), so I'm moving it back to Drupal 6.
I think this narrows it down to one very likely culprit: the "split summary at cursor" button on the Body field, which currently exists in Drupal 6 but no longer in any other version of Drupal.
Comment #18
David_Rothstein CreditAttribution: David_Rothstein commentedActually, it looks like it's still reproducible in Drupal 7, but only with Internet Explorer - all other browsers seem to work fine. Aargh...
Comment #19
casey CreditAttribution: casey commentedhttp://stackoverflow.com/questions/261351/browser-back-button-restores-e...
http://stackoverflow.com/questions/1485672/losing-form-data-when-clickin...
http://blog.httpwatch.com/2008/10/15/two-important-differences-between-f...
Comment #20
Bojhan CreditAttribution: Bojhan commented#655388: Many ways to lose data on form input in the overlay
Lets not duplicate efforts.
Comment #21
casey CreditAttribution: casey commentedComment #22
Bojhan CreditAttribution: Bojhan commented@casey please, add some information when you do stuff :P jeez
Comment #23
casey CreditAttribution: casey commentedSorry :)
The other one is just about the overlay and only contains a fix for forms inside the overlay.
The real issue is much broader (and causes issues like #358961: checkboxes lose state if action requires a confirmation screen and user cancels or uses back button). According to the link I posted it may be caused by cache headers. I'd like to see if we can fix that.
Comment #24
Bojhan CreditAttribution: Bojhan commentedBut its not really only related to the create content page then?
Comment #25
casey CreditAttribution: casey commentedNot indeed.
Comment #26
David_Rothstein CreditAttribution: David_Rothstein commentedWhen the cache headers were previously incorrect it caused much wider problems with forms being emptied all over the place: #109941: Store site view in HTTP headers
I guess it could still be the case that some minor adjustment is needed to what we have now to make IE behave correctly, though.
Comment #27
casey CreditAttribution: casey commentedThis should do.
Note this isn't working inside overlay cause of the way overlay is working. I might be able to fix that however. If so I'll post a patch in the overlay-specific issue.
Comment #28
catchLooks interesting, will try out later.
Comment #30
casey CreditAttribution: casey commentedFixing test.
Comment #31
locomo CreditAttribution: locomo commentedsubscribe
Comment #32
xmacinfoComment #33
Alex UA CreditAttribution: Alex UA commentedI'm bumping this to "major"- we're getting a bunch of complaints about this behavior in 6.x from our clients, which I know is a self-selecting/unscientific polling method, but I have to believe this is causing a lot of pain to Drupal site owners and is really a pretty nasty little bug (borking the browser = bad).
Comment #34
mgiffordI'm moving this to D8 so it can be looked at and hopefully backported.
I'm also interested in if there are suggestions to Seven where this can be applied in the interim.
It's a really frustrating problem and hopefully it will be fixed in browsers, but this one bit me today in Chrome. Wasted a bunch of time editing a blog.
Comment #35
casey CreditAttribution: casey commented#30: cachecontrol.patch queued for re-testing.
Comment #36
sunIndeed interesting. Love the comments. However, this is not major.
Comment #37
mgiffordMy team upgraded this module to D7 and it works great - http://drupal.org/project/node_edit_protection
It would be great if there was something like this as part of core in D8 mind you.
Comment #38
cosmicdreams CreditAttribution: cosmicdreams commentedCould we use this technique: http://net.tutsplus.com/tutorials/html-css-techniques/quick-tip-learning...
Comment #39
mgiffordHTML5's local storage? I suppose that might work. Should be better supported by the time D8 is released. I think this is likely much bigger than this particular issue queue item mind you.
Comment #40
geerlingguy CreditAttribution: geerlingguy commented@mgifford - I'll be testing Node Edit Protection to see how it works for one of my sites (looks to be very similar to Dirty Forms and onBeforeUnload, also mentioned in this issue).
I've been getting these complaints from site users here and there... it's mostly an annoyance, but enough of one that I will be looking for better ways to solve it. local storage would be one nice way (HTML5), but that still won't work well for most people for another 2-4 years, I think.
Subscribe.
Comment #41
mgiffordHopefully Node Edit Protection works for you. It's a simpler implementation I think than having to enable Dirty Forms & onBeforeUnload which seems to be the proposed approach.
If folks like the approach used in Node Edit Protection we can look at producing a patch for core.
Comment #42
nod_tag
Comment #43
nod_Probably needs a reroll, just saying :)
Comment #44
mgifford#30: cachecontrol.patch queued for re-testing.
Comment #46
johnennew CreditAttribution: johnennew commentedHi all,
PeteB, ChrisH and I supply the attached patch for review. It's certainly a work in progress but needs some initial comments now. We're using local storage to store form values as they are entered. When the form is revisited the previous values are recalled from localStorage. If the user deliberately closes the overlay with its close button or saves the form then the local store is removed.
This implementation fixes the specific use case described previously, the steps to reproduce are:
1. Click add new article
2. Fill in the form
3. Press Preview
4. Click "Read More" or otherwise leave the overlay without clicking save or close
5. Press back
Previous outcome: the form would be empty
Outcome with this patch applied: Your previous form states are restored.
Comment #47
Bojhan CreditAttribution: Bojhan commented@ceng Thanks, awesome that you guys have been working on this.
Comment #48
johnennew CreditAttribution: johnennew commentedHeh, already found it doesn't work in Chrome, attached is a better version.
Comment #49
nod_That's a pretty good start :)
It'd be good to read up on http://drupal.org/node/172169
As for the patch itself, a new file like form.autosave.js would be better than stuffing it in form.js.
you can use document.querySelectorAll instead of getElementsById, getElementsByClassName
Drupal form elements will always have an ID (or they should at least) you can based you assumptions on it, the serialized object can be a simple {id: value, id2: value2} kind of object (we want to keep the data as light as possible for storing in localstorage).
Just use jQuery for updateForm. No need to spend time to work around at this point :)
Don't hesitate to create new events that will be listen for to clear the localStorage. It's much better than hardcoding the click thing on relevant DOM elements (what about keyboards right now?)
So yeah, it's a good start! thanks!
Comment #50
johnennew CreditAttribution: johnennew commentedHi all,
Still some work in progress but would be good to get a review at this stage. I've followed nod_ suggestions in #49
* Applied coding standards
* Moved the form save javascript to form.autosave.js
* Added code to the system module to load this js when forms are used
* Added code to the node module to place a class on a form which want to save form state immediately when loaded, for example when previewing or after a form validation error (still to do) as in these cases the users form state has not been saved on the server.
* Added an event called drupalOverlayCloseByLink which is fired when the overlay is deliberately closed. We should remove the localStorage version at this point
* Removed the timed saving of the form to localStorage and replaced with saving on a formUpdate event.
* Replaced some of the items with jQuery instead as suggested.
Do all instances of getElementById need replacing with querySelectorAll?
Comment #51
mgiffordI got this on a fresh install:
I think that this form.autosave.js is not stopping me from loosing my data if I navigate away from the page in FF on Linux. Haven't done more testing.
Comment #52
johnennew CreditAttribution: johnennew commentedThanks for the review mgifford!
I've not got a local linux install but I'll setup a Virtual Box with CentOS to give it a go.
I fixed the whitespace issue in the patch but I don't understand the error 'error: core/misc/form.autosave.js: already exists in working directory' - it is this patch that creates this file so there is no way it would be there already. I tested the attached patch on a fresh d8 install and I don't get either error now.
mgifford - when you say you don't think this patch solves the issue, could you describe the steps you take, what you see and what you expect to see (or reference comment above you think is not being achieved); as far as the workflow in #46 goes, I believe this is solved with this patch. Are there any JS errors in your console?
This new patch adds some additional logic to not load the localstorage version of the form if the form's changed timestamp has changed since the local storage copy was made. This means that if someone else updates the form to the server, your local changes will be lost next time to arrive and you'll see the current latest version of the form. I think this makes sense.
Comment #53
mgiffordDon't worry about the form.autosave.js... I'm thinking that may have just been there from a previous install. After deleting the file, then resetting the repo it applied without difficulty.
Comment #54
johnennew CreditAttribution: johnennew commentedAnother patch update, added support for select elements. Also removed a floating console.log in the last patch (doh!)
Still todo:
Comment #55
johnennew CreditAttribution: johnennew commented[bad patch]
Comment #56
johnennew CreditAttribution: johnennew commentedHi all,
I think this patch now does everything required of it but needs a close review of coding standards and approach. I added a Array.indexOf prototype (see below) which might well be done in a more elegant way:
Also, there's some suggestion of enabling the autosave on a form in a better way than setting a class on the form, perhaps through some formapi method.
Kind regards,
John
Comment #57
Angry Dan CreditAttribution: Angry Dan commentedHi ceng,
Rather than adding stuff to the prototypes of JavaScript objects (John Resig once said this was bad, can't find the post now but he definitely said it somewhere) it would probably be a better practice to define a function for indexOf(). But, since jQuery already has one, you might just want to replace your calls to Array.indexOf with $.inArray(). See http://api.jquery.com/jQuery.inArray/ for documentation on that.
Also and I don't fully understand this yet, so @_nod might be able to help out here, but would this be better implemented using the Drupal.settings object? I'm guessing the ideal would be to do something like this to your $form array:
(Here's where I get shaky) I'm guessing that then you would use
hook_form_alter()
to catch all forms with the autosave property set to true. Then you coulddrupal_add_js("autosave.js", $settings)
where $settings contained an array of form ID's to be autosaved. Then in your drupal.behaviours I believe the form ID's would be passed directly into your behaviour code.Any thoughts?
Comment #58
nod_Great work, that's awesome :)
I do have some things to point out.
$('form').formToArray()
method, using it would turnDrupal.form.localSave
in a 2 line function. Some data structure changes but nothing too crazy.Sorry it's just a short review, it's very much on the right tracks, keep it up :)
Comment #59
nod_There are still issues with fields in vertical tabs, doesn't save everything.
There should be a way to discard localstorage values and go back to the default values. I guess it's the revival of the reset button :p Also we might want to use sessionStorage instead. This is the backbutton issue after all, not the 'everything crashes' one.
Comment #60
johnennew CreditAttribution: johnennew commentedHi nod_
Not sure I understand exactly what you mean in #59.
Vertical tabs: I'm on chrome and ff on a mac and can't replicate this issue, what field values are not saved or restored from a vertical tab? I tried all the basics on a node edit form - sticky at top of lists, URL path, comments settings, create new revision, log entry etc. Clicking off the form then pressing back appears to restore the values I entered here fine.
Reset button: You should see a "Clear local edits" button in the message that appears at the top of the form when local edits have been applied, this button disappears onces clicked and so I think is preferable to the form reset button. (see screen shot attached)
Back button: Using localStorage rather than sessionStorage solves both the crash issue and back button issue. sessionStorage is just back issue. Isn't this approach therefore preferable? I notice that basecamp opts for the localStorage approach for saving comments in a thread so you can go away and come back and your last edit is still there waiting for you.
Will look at implementing the suggestions in #58 next though I am going to spend some time on dan-sprog's suggestion in #57, I wonder of this approach is more in keeping with the rest of formapi, so adding a simple parameter like this includes the JS file?
Comment #61
Bojhan CreditAttribution: Bojhan commentedWhat is this local business, no user knows what that means.
Comment #62
johnennew CreditAttribution: johnennew commentedHi Bojhan, thanks for your message. Wording is easy to change, could you suggest something better?
Comment #63
Bojhan CreditAttribution: Bojhan commented@ceng I am not sure what exactly it does, so I might be wrong here - but what about just removing local: "You have edited this form before, so it has changes which have not been saved yet"
I don't think the word "edits" is a very clear label, I'd go with changes. For the button a couple options:
[ Reset previous changes]
[ Reset ]
[ Remove earlier changes ]
[ Clear previous changes ]
Comment #64
seutje CreditAttribution: seutje commentedshould have some time later today to take this out for a spin. Looks pretty solid already though.
Comment #65
seutje CreditAttribution: seutje commentedwhy are we adding this class and shouldn't it reflect the name of the behavior at least a bit?
we don't seem to be using these parameters, how do we handle ajaxy forms?
could we cache these queries?
we don't seem to be using these parameters, can we leave them out, or at least use the same naming?
should probably guard against broken prototypes with hasOwnProperty. And I would be more comfortable not touching the Array prototype and use $.inArray instead, or at least have the polyfill live in its own file.
please don't use soft comparison with type coercion, and is there any need for the inner parens on typeof?
this doesn't work, does it? the summaries on vertical tabs don't seem to get updated in my chrome. You probably need to trigger formUpdated on the field or summaryUpdated on the fieldset.
Comment #66
seutje CreditAttribution: seutje commentedhmz, when I click the "clear local edits" button and refresh, the local changes are still reset, but the message also still appears and I can "clear local edits" again. (chrome 22 on OSX, but I don't think it matters)
Comment #67
__cj CreditAttribution: __cj commentedThis patch removes the indexOf prototype, and replaces it with $.inArray() (as suggested in #57), caches some queries (as suggested in #65), and replaces 'edits' with 'changes' (as suggested in #63).
Comment #68
mgiffordgo bot.
Comment #69
johnennew CreditAttribution: johnennew commentedThis patch builds on __cj's patch. We've removed the use of classes to define that a form should be autosaved and replaced by a new formapi attribute:
$form['#autosave'] = array(
// Set Enabled to true to tell JS to save the form state in localStorage.
'#enabled' => TRUE,
// Set presave to TRUE to save the form values to localStorage as soon
// as the form is loaded. This is useful after a validation error or on presave
// when the form was not saved on the server.
'#presave' => TRUE
);
The message informing the user there are local edits is now:
"You have previous changes which have not been saved yet." and the button text to clear local edits is "Discard previous changes"
Thanks for all the comments, we're still working through them all,
John
Comment #70
yurtboy CreditAttribution: yurtboy commentedPatch applied fine and the only field that did not save values for me was the Tag field. I typed in a tag that did not exist, click a link on the menu, pressed back and it was gone but all other fields where fine.
I then made a term on the Taxonomy page. Made a new node and referenced this new term. But when I clicked a link in the menu then the back button this was the only field still not saved.
Firefox 16.01
Comment #71
xmacinfo@yurtboy: Looks like you can switch back this issue to Needs work. :-)
Comment #72
seutje CreditAttribution: seutje commentedWhen adding an article, I also seem to be getting an error on line 172 of form.autosave.js (
var type = el.tagName === 'INPUT' ? $(el).attr('type') : el.tagName;
) because thatel
isnull
when it tries tovar el = document.getElementById(key);
wherekey
is'edit-field-image-und-0-alt'
Comment #73
nod_I would very much like to have this for D8, meaning we need to be rtbc in a matter a weeks.
@ceng, would you have some time to follow-up on this? I'm happy to give reviews or unblock whatever is needed to make it before feature freeze.
Comment #74
barraponto CreditAttribution: barraponto commentedCould not reproduce #70 running Firefox 16.0.2 nor Chrome 23.
Using overlay, btw.
Comment #75
barraponto CreditAttribution: barraponto commentedSuccesfully reproduced #72, Firefox. Keep in mind you have to preview in order to see the error.
I found another issue: once you come back and the warning message is displayed, uploading a file in a file field widget makes the warning show up again. twice!
Comment #76
johnennew CreditAttribution: johnennew commentedThanks for the reviews, I should have time to look at this on 23/11
Comment #77
barraponto CreditAttribution: barraponto commentedSo, #72 seems to happen only with filefield widgets, maybe with any field that is not displayed by default. I got around by checking whether
el
is defined, since I definitely don't think we can save file input values anyway. Gotta see what happens with multiple valued fields though.Comment #78
johnennew CreditAttribution: johnennew commentedHi barraponto - tried out your patch and seems to be working as far as I can tell. I think you are right to say you cannot set a file input element with javascript.
I've added a new '#changed' option to the autosave form element. This lets form builders specify the last changed state of the form or entity the form is altering. If someone else edits and saves a node back to the server we want all local copies to be discarded - they have all been over written, the same as would happen if two people edited a node at the same time, only one would end up saving it.
Ready for further review.
Comment #79
johnennew CreditAttribution: johnennew commentedAnd here is the patch.
Comment #81
Angry Dan CreditAttribution: Angry Dan commented#79: drupal-153313-form-data-back-button-6740346.patch queued for re-testing.
Comment #82
Angry Dan CreditAttribution: Angry Dan commentedNot sure why the last patch failed testing, but it's passed now.
Patch all looks good to me!
Comment #83
barraponto CreditAttribution: barraponto commentedWe still need to address the issue i mentioned in #75:
Comment #84
LewisNymanAfter a quick chat on IRC with Wim and nod_ I think it's worth suggesting we leverage Garlic.js to persist the local data. It seems well tested, full featured, and with a decent amount of community support. It would give Garlic.js a nice boost is test cases if we threw our weight behind it.
What does everyone else think?
Comment #85
barraponto CreditAttribution: barraponto commentedThe less I have to mantain, the better.
Comment #86
mgiffordI hadn't heard of it before, but http://garlicjs.org looks good to me. Wow - "don't lose any precious data if they accidentally close their tab or browser" - that's a big feature enhancement over simply protecting against just hitting the back button.
Comment #87
LewisNymanOk, I've got a proof of concept patch up and running with Garlic.js. Apply and try out on any form!
I've kept form.Autosave.js that requires Garlic.js. At the moment it just calls Garlic against all forms on the page. I couldn't seem to get the Drupal.settings output and that's really not my strong suit, I'm hoping someone can help merge the two patches into a more complete solution.
Comment #88
moshe weitzman CreditAttribution: moshe weitzman commentedMore ambitious title :)
drupal_add_js($form_autosave_settings, 'setting');
. should use #attached['js'] instead.Lets think about what tests we could add. Might be none.
Comment #89
Wim LeersComment #90
Wim LeersThis reroll is about cleaning up and conforming to latest coding standards (including #88). It now also properly uses
drupalSettings
(which was not yet done as per #87).1. It supports expiration (imagine the megabytes of text core contributors have entered over the course of months on Drupal.org, our localStorage would grow quite large :)), yay! The patch at #79 did not yet do that.
2. As always with these things, it's in the conflict management aspect where these things either shine or suck. Garlic is in between. It does not support a server-side generated ID and depend on that unique ID to automatically discard (or at least ignore) localStorage data. We could do trickery with comparing the "changed" timestamp with the auto expiration date stored in localStorage, but that's never going to be reliable (timezones etc), we just need it to accept whatever ID we feed it.
3. If I look at the data in localStorage, it is stored with a key such as
garlic:localhost/d8/node/1/edit>form:eq(0)>input.field_image[und][0][alt]
, despite me passing using the form ID as a selector. Clearly, Garlic.js fails to differentiate between multiple forms on the same URL with similar form structures that can change order. But then again, which forms would swap order *and* have the same form structure? Still, I'd rather see this use the form ID, if it's available.Taking the above into account, both remaining problems (points 2 and 3, point 1 is no problem) are solvable by a simple change: improving garlic by allowing implements to implement a callback that will generate the "root node" part of the DOM structure of the localStorage key. I.e. instead of
form:eq(0)
, we could then generateform#article-node-form[data-localStorage-changed="<whatever value we have for 'changed'>"]
. That would still require us to set thedata-localStorage-changed
attribute (in JS, before calling.garlic()
, which would not invalidate Drupal's render cache) though, but it would require only minimal changes to Garlic.js.I don't think the
#presave
stuff is necessary with Garlic, though it's also simply not entirely clear to me why it's necessary. AFAICT it's only used for the message to alert the user and to give them the option to get rid of their previous changes. Is this really necessary? I can definitely see its use, but I think it might be annoying to be grI also don't think the
enabled
flag makes any sense to pass to the JS side; if it's not enabled, then just don't pass it viadrupalSettings
. Similarly,['#autosave']['#enabled']
doesn't make any sense: to not enable autosaving, simply don't set (or unset)#autosave
.Comment #92
Wim LeersA tiny remnant of my CKEditor core work snuck into #90. Sorry.
Comment #93
Sutharsan CreditAttribution: Sutharsan commentedChanging state to 'needs review'. I guess that was intended ;)
Comment #95
Pete B CreditAttribution: Pete B commentedReroll of #92 to make it pass testing...
Comment #96
Wim LeersI'm pretty sure that the problems I pointed out in #90 have been solved upstream in Garlic.js 1.2.0, see https://github.com/guillaumepotier/Garlic.js/commit/aa6af62254fc2a3818ca....
Anybody who wants to push this to the finish line? Feature freeze is next Monday!
Comment #97
Wim LeersStraight reroll of #95 to apply to HEAD again.
Comment #98
Wim LeersI'll take a quick stab at this, because it shouldn't be too much work.
Comment #99
Wim LeersThis should be close to committable.
Fully working. All concerns I expressed in #90 are addressed, and loose ends have been cleaned up.
Comment #100
netsensei CreditAttribution: netsensei commentedOkay. I skimmed through the documentation and the patch, but I didn't have the time to test it. I do have one minor question based on the patch in #99:
Would this enable autosave only on the node edit form? My first reaction was: what about other entities? The taxonomy term could have multiple fields. There are use cases where i.e. the description field might contain more text (i.e. Detail page of a taxonomy term containing a title, description and a list of referenced entities)
Moreover, I think it would be inconsistent if the node edit form has this safety net while other entities created by content managers,... don't.
So, I was wondering: if we'd move this to EntityFormController instead, wouldn't all entities profit from this?
Comment #101
Wim Leers#100: That's probably the only remaining code from before garlic.js (before I started working on this) :)
You are of course right, but I figured we might want to slowly introduce this. For nodes, it's obvious that it's useful. I think we should discuss the consequences of enabling it for all entities (including comments) in a follow-up issue.
Comment #102
jessebeach CreditAttribution: jessebeach commentedI pulled
generateGarlicLocalStorageKey
off ofDrupal.behaviors.formAutoSave
and moved it to theattach
method's closure so it can't be futzed with.I tested nodes and it works as promised. Even across tabs which is really sweet!
I'm not in love with Garlic, but I also wouldn't want to rewrite it. So Garlic it is.
Comment #103
Ranko CreditAttribution: Ranko commentedIf you don't like Garlick.js how about Sisyphus.js http://simsalabim.github.com/sisyphus/ ? In other words, what's the issue with garlic?
Comment #104
jessebeach CreditAttribution: jessebeach commentedI just found the code in the Garlick source incredibly hard to read because of its formatting. Who puts a comma at the beginning of a new line?
Ranko, that's an interesting comparison! I brought it up with Wim Leers and he thinks it's worth a comparison. So I'm evaluating it now. Thanks for the suggestion!
Comment #105
jessebeach CreditAttribution: jessebeach commentedSo, here's my take after looking at the two code bases and the two projects.
Let's stick with Garlic for now
Why?
We can revisit these thoughts later, but for now, Garlic seems the better choice.
Comment #106
netsensei CreditAttribution: netsensei commentedJust tested the patch with the default Article content type.
1.
I noticed that the field_image upload is not restored. Neither before or after the upload button has been pressed. Its value does get stored as
garlic:node/add/article~0>form#article-node-form input#edit-field-image-und-0-upload
in Local Storage however.As far as I understand garlic.js and what it does, Garlic doesn't really discern between types of
<input>
elements. It treats the file type as any other text field. So, I believe this more a problem on the side of Garlic then it is with Drupal.My question: Should we keep the reference to a file on the users' machine in localStorage if it isn't going to be used anyway?
2.
On a related note: when I uploaded a file, I filled out the "Description" text field. The field is triggered through AJAX but I noticed that it doesn't get stored. Given my previous point, the upshot is that any stored "description" field data can not mismatch if the user would add a different file instead. On the flipside, form altering field widgets like multivalued or dependent fields (i.e addressfield, field collection,...) might prove hard to restore to their original state. (I set the body field to multivalued, add a few values, closed and reopened the page: only the first item gets restored. Adding a second item to the field does not restore the original values)
Then again, I take Garlic for what it is: a tool for restoring form data, not form state. Unless it's our intention to broaden the scope and restore state too, these objections are minor.
3.
Do we need specific tests for this?
Apart from that: the patch looks really solid, cleanly applies and works for me.
Comment #107
Wim LeersSee also:
(Source for all of the above: http://my.opera.com/community/forums/topic.dml?id=685752&t=1361097892&pa... — transcribed here for posterity.)
Solved by excluding
input[type=file][type=hidden]
from the inputs that Garlic.js should handle (I figured I'd explicitly exclude the hidden<input>
s at the same time).#states
system, those should work just fine, since the form item already exists.Assuming this patch gets RTBC'd, follow-up issues for these aspects should be created:
#autosave
, besides on the node form?Comment #108
nod_Broken on FF
Comment #109
nod_garlic.js is a massive cpu hog on FF at least, makes FF take 10-20% of CPU when idle
Comment #110
Wim LeersI can't reproduce because FF is using 7–15% when idle, without a single page loaded. This also makes it effectively impossible for me to debug this. :(
Comment #111
sunI can't comment on the browser-specific performance issues today/right now, but here's my review of the Drupal integration code (which most likely won't change, regardless of which library is used):
drupal_process_form()
isn't really the right spot to for #attached - the info is only needed when a form is actually rendered.Technically, the most appropriate place is a new #pre_render callback that's declared for #type 'form'.
We're not using ctype functions in Drupal.
Thus far, the only used #autosave identifiers are timestamps and numbers? So any reason to not just simply cast to (int)?
Must be namespaced correctly; i.e., data-formautosave-changed
Hmm. We can make a lot of assumptions about Drupal forms, but HTML IDs on the form or on form elements do not actually belong to them. In fact, the most important form input elements in a Drupal form do not have a HTML ID (i.e., the hidden form_id, form_build_id, token, etc elements).
Does that mean this facility will suddenly forget my last input if you happen to comment on a d.o issue before I managed to press submit?
Why would the server-side state of an entity play any role in this? Isn't the whole point here to auto-save form data?
Note: Garlic seems to have some limited support for handling conflicts.
What if $node->changed is 0?
I'd prefer a dedicated test class for this functionality.
This does not actually test #autosave = FALSE/NULL. It only tests a non-existing property.
Comment #112
Wim Leers#111:
Thanks for your stellar feedback! That's the sort of feedback I'd been looking for for a while :)
Most concerns addressed; where relevant or where not addressed, explained below:
I tried precisely this, but … it doesn't work for entity forms, seemingly because
entity_get_form()
callsdrupal_build_form()
, which doesn't calldrupal_render()
, and none of the code in form.inc has handling for#pre_render
; that is adrupal_render()
thing.I used
#process
instead.I wanted to allow for alphanumerical identifiers. But you're right, just integers should do.
Note that your claim is not entirely true. I did check whether Drupal was using ctype, and it actually is: in
\Drupal\Core\Utility
and in the EasyRdf, Assetic, PHPUnit, Symfony, and Twig libraries.Well, we don't need to store
input[type=hidden]
' values anyway; they're explicitly excluded in #107.I think that for the remaining form items, this assumption is true? I.e.,
form_builder()
does this:AFAICT, that would cause every non-hidden form item to have an ID?
Great point. I've also pondered about this.
It seems like you're conflating two things here though:
$form['#autosave'] = $node->changed
$form['#autosave'] = $comment->changed
, not$form['#autosave'] = $node->changed
. (I've added autosaving to comment forms in the attached reroll.)I agree it would make no sense at all to have your locally saved comment being deleted (actually, ignored, it's currently left to garlic's expiration mechanism to do the clean-up) if the node had changed. But it does make sense (in most cases) to have your locally stored node/comment form data be ignored if the node/comment has been updated since you wrote it. After all, your modifications may no longer be relevant.
OTOH, for things like issue summaries, where multiple people might be making modifications, it is definitely reasonable to want to have a way to regardless of changes made by others, still being able to retrieve your changes. But that implies introducing yet another UI concept, with a most likely confusing UI — see Bojhan's comment in #61.
So, personally, I'd rather defer deciding how to deal with that to a follow-up issue. In that follow-up issue, we could then decide whether core should handle this, or whether it should be left to contrib.
In any case, this is an improvement and a big step in the right direction.
#108/#109:
If this really were a Garlic.js problem, then:
Nevertheless, I tightened Garlic.js settings:
Garlic.js works fine for me in Firefox (tested by going to node/add/article, typing stuff everywhere, closing Firefox, opening it again, navigating to node/add/article again, and it's all there).
(Maybe the tightened settings helped.)
If you're still having problems, please disable EVERY Firefox add-on, manually reset all caches and restart it. Possibly create a temporary "clean slate" new user profile. Firefox (18) exhibits a lot of weird performance characteristics, so it's possible that that is also influencing what you're seeing.
Besides the above:
Comment #113
msonnabaum CreditAttribution: msonnabaum commentedPerformance impact looked rather minimal to me doing a quick profile in chrome on a node add form, but I didn't try on firefox.
The patch looks rather good to me. The only thing I noticed was that generateGarlicLocalStorageKey is defined globally and I'd expect it to be somewhere on the Drupal object.
Comment #114
Wim Leers#113: actually, it's not at all a globally defined function, it's scoped in a closure (meaning you have no way to modify it at all). :) Jesse did this in #102. (Originally, I had it at
Drupal.behaviors.formAutoSave.generateGarlicLocalStorageKey()
.)Comment #115
Dave ReidDoes this work with WYSIWYG-enabled fields now that CKeditor is included in core? Has anyone tested that? I don't see any issues in the Garlic.js tracker or documentation related to WYSIWYG.
Comment #116
jessebeach CreditAttribution: jessebeach commentedIt seemed to me that we don't want to make the function public to alter. If a contrib module comes along and changes the function and subsequently how the local storage key is generated, it could strand a lot of saved data in localStorage.
So as the patch stands, the function is tucked away in a closure where outside scripts can futz with it.
Comment #117
msonnabaum CreditAttribution: msonnabaum commentedYup, that seems the right thing to do. Carry on :)
Comment #118
Wim Leers#115: Wow. Great point. I think we've all been so focused on getting this to work well, that we forgot about WYSIWYG :)
The short answer is: yes, this is possible.
The better answer is: here it is :)
At first, I was hesitant to do it in this patch — I wanted to make it a follow-up. But both Dave Reid and msonnabaum preferred to get that committed as part of this patch, so here we go.
The interdiff will show you how little has changed: A) editor.module's JS API is extended slightly to also have an
onChange(element, callback)
method (was already planned and reviewed at #1886566: Make WYSIWYG editors available for in-place editing + #1873500: CKEditor + Edit anyway), B) editor.module will now serialize changes every 0.5 seconds back to the corresponding<textarea>
and will trigger an event, C) garlic.js will now listen to that event as well.EDIT: and to clarify: I checked with msonnabaum on the "sync changes every 0.5 seconds" aspect, and that definitely does not introduce performance problems; nor would it lose to much user changes — after all, how much can one type in 0.5 seconds? :)
Test it at http://simplytest.me/project/drupal/612c7e2c4b79927b919175c0eaeb115ab6172215?patch[]=http://drupal.org/files/form_autosave_garlic-153313-118_0.patch.
Comment #119
seutje CreditAttribution: seutje commentedran trough the code and don't see anything I'd have a problem with, also tested it manually on a variety of setups and could manage to break it or experience any noticeable performance issues.
I wasn't able to test it on IE10, and considering https://github.com/guillaumepotier/Garlic.js/issues/25, I was wondering if anyone had an IE10 environment they could manually test this with?
Comment #120
Wim LeersI do not; but considering IE10's much improved standards support, I expect it to run just fine. In any case we can solve browser-specific problems if they arise, and thanks to Garlic.js' test suite, it should be pretty easy to prevent regressions.
What's left to do to get this to RTBC? :) We've got a few more hours left to achieve that…
Comment #121
webchickI don't think this is a commit-blocker, per se, but UX-wise, it would be nice to get a dsm() or some other kind of alert when things are being loaded from the local store. A real, actual use case that just happened:
I applied this patch, edited an article and started typing crap on the end of my tag to see what would happen. Closed the browser. Then I got into an IRC PM session for a half hour. Then I ate lunch. Then I came back to test something related to WYSIWYG, completely forgetting about the earlier edits I'd made and the patch I was testing, and I had random crap on the end of my tags, which was not there on the front end. What?
It would be nice if, upon loading the node edit form, I got a little message that was like "Loaded your changes from an earlier draft" or something. Except not draft, since it's not a DRAFT draft. Ugh. Is there other CMS's text out there we could copy/paste?
Comment #122
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis is a really bad idea, as there are many ways it can and will be wrong.
Two examples off the top of my head (I haven't checked if it is true, but seems logical conclusions from reviewing the code):
Given the current state of the Form API, in-progress form data will have to be saved on the server side, we cannot reliably save anything on the client side.
Comment #123
Wim Leers#122:
<input type="hidden" name="changed" value="<$node->changed value here>">
, because that's ainput[type=hidden]
, which we specifically do not deal with. So even if we weren't going to ignore form data when the node was changed, then it would still be impossible to have an endless lock.Your specific use case would work like this: user B has saved the changes, user A tries to save but gets the "modified by other user" message. Upon reloading the page, the form will actually no longer contain any of the changes.
Please see #112's reply to #111, point 3 (the reply to the blockquote) for full detail.
This was originally raised in #106.2, and I answered in #107.2:
Restoring of AJAX form items is nigh impossible, because it'd require replaying user actions in certain cases. So yes, from that POV you're right that server-side storage is a requirement. But that comes with increased network activity (and thus worse battery life), scalability/storage requirements, poor reliability on mobile/spotty network connections and so on. So that's actually a really bad idea as well.
I'm a perfectionist, so I totally understand where #122 is coming from. This patch isn't going to be able to cover every possible advanced form use case, that's true. But it's going to make a significant difference for big parts of much used forms.
Given that the first point was wrong, and the second one is also "a really bad idea", reverting to needs review.
Comment #124
swentel CreditAttribution: swentel commentedDoesn't work indeed, even with a single image upload for instance either. I can predict dozens of support/bug tickets in the field queue to ask where 'OMG, where did our uploaded files go to, or my second textfield, or whatever". The only answer to that will be "won't fix" or "works as designed" from me (and I think yched would agree). I know that sounds negative, but I can only give you my point of view as well on this.
Comment #125
swentel CreditAttribution: swentel commentedCrosspost - none the less, I don't think this is a good idea.
Comment #126
jessebeach CreditAttribution: jessebeach commentedI don't understand why we're unwilling to make a minor improvement that will save some data because we haven't yet made the major improvement that will save all data.
An uploaded image can be re-uploaded. A textarea of text that gets lost is gone. Gone like a thesis lost to a failed hard drive. That's harsh.
This issue had been active for more than 5 and a half years. That's 2007 days of people losing the body text of a node they just spent perhaps hours writing because when switching tabs they accidentally closed the tab instead of selecting it.
An incremental improvement here is better than no improvement.
Comment #127
jessebeach CreditAttribution: jessebeach commentedI think I cross-posted to needs work. Setting back to needs review.
Comment #128
msonnabaum CreditAttribution: msonnabaum commentedIt's a shame this won't work with ajax forms, but I think this is most useful on simpler forms with textareas anyhow, so no sense in vetoing the whole idea based on that.
I'd be fine if this didn't have the DSM, but it might be nice to clarify that Drupal restored your content and not your browser. That could easily be handled in a followup.
I tested this again with a more complex content type and it worked quite well. This seems ready to me.
Comment #129
Crell CreditAttribution: Crell commentedFor fancy ajaxy forms, there's still the autosave module. (Much to my dismay. :-) ) We could probably borrow some UI ideas from there for the "hey, you've been restored!" message, too.
Comment #130
jessebeach CreditAttribution: jessebeach commentedI'm happy to code the followup that produces a message to a user that form data has been restored. Bojhan and I can collaborate on what that would look like.
Comment #131
sunWe still need to use #pre_render instead of a process callback.
That said, I also agree with @Damien Tournoud in #122. If you fully think through the consequences, then we essentially arrive in the same problem space and conclusion as #1510544-131: Allow to preview content in an actual live environment — user data should be saved on the backend instead.
I don't really see the point of adding a facility to core that claims to do something and thus introduces user expectations, while the facility is known and guaranteed to break.
As long as that rather big limitation exists, this is contrib material.
Comment #132
webchickTagging as something that was RTBC in time for feature freeze.
Comment #133
Damien Tournoud CreditAttribution: Damien Tournoud commentedI agree with @sun that something that works in some very limited cases (in my experience, 99% of the node forms have an AJAX "Add another item") is contrib material.
Comment #134
Gábor HojtsyAs said above, entity forms don't use any of the code that would currently make it possible to use a #pre_render. We can go and refactor entity form handling, that might not be wise to do in this issue.
Comment #135
netsensei CreditAttribution: netsensei commentedI tend to follow Sun et al per #106. Given the extensive efforts put into UX through other initiatives, I think this boils down to: are we willing to compromise for the sake of a moderate functional improvement?
Come to think of it: let's not forget that this is problem is not limited to Drupal. Any filled out data on any form on the web is prone to getting lost. Other frameworks, like WordPress, solve this with similar basic autosave functionality. Our ingrained AJAX system in the Form API really raises the bar to get this in in a sensible way on short notice imho.
Comment #136
Crell CreditAttribution: Crell commentedAutosave gets around the dynamic form problem by just regenerating and replacing the entire form server-side rather than using localStorage. It's horribly ugly, but works. If this does get punted from core this cycle I'm happy to take the work here into autosave to replace what's there. (I'm happy to turn it over to someone from this issue entirely, in fact. :-) )
What about allowing the client side code to essentially re-script the the process of modifying the form? Probably something more elegant than a macro, but along those lines: FAPI demands that we trigger "change the form structure" commands. So, let's trigger those commands, then fill in the saved data accordingly.
Comment #137
Damien Tournoud CreditAttribution: Damien Tournoud commented@Crell: the whole state of the form is on the server. There is no way to reconstruct it completely from the client at this point. We should should refactor the Form API so that (1) the state is the king, and the form is built from the state [which is kind of the case right now, but only kind of], (2) interaction to the state is done via actions (as in MVVM models). This would allow us to build the form both from the client and the server, to perform some of the form modification / validation workflows directly on the client without hitting the server, and eventually to save the full state of the form in the client, as described in this issue, while keeping a unified framework between the client and the server and strong validation guarantees.
But that's a complete other story :)
Comment #138
Crell CreditAttribution: Crell commentedDamien: Yes, that's very Drupal 9 material, wherein we may be looking at the Symfony Form API anyway. For Drupal 8, is what I described possible? Conceptually I don't see why it couldn't be, even if it's ugly.
Comment #139
moshe weitzman CreditAttribution: moshe weitzman commentedI think this is a significant usability win, and am fine with reuploading images, not supporting multiple values, etc.
Comment #140
nod_We're way past feature freeze and well into API freeze.
Comment #141
kattekrab CreditAttribution: kattekrab commentedBut this was RTBC before Feature Freeze... see #132
and agree with @jessebeach #126
Comment #142
Wim LeersMoving back to 8.x because it is not uniformly agreed upon that this should be a 9.x feature.
Furthermore: this is a small *addition*, not a change, that could potentially happen in e.g. the Drupal 8.2 release.
Comment #143
Damien Tournoud CreditAttribution: Damien Tournoud commentedRandomly marking a feature RTBC before feature freeze doesn't make it actually RTBC (granted, that would make the review process way easier if it was true).
We have three people (sun, swentel, myself) agreeing that the feature is at most half-backed in it's current state, and will require significant refactoring to get right. So, yes, this is Drupal 9 material.
Also, this is really not a bug for any definition of a bug.
Comment #144
moshe weitzman CreditAttribution: moshe weitzman commentedAgree on this being a feature.
Please don't remove the tag that Angie set in February. We should let the committers decide about eligibility for D8. If they decide that now is too late, then the supporters will work on this for D9.
We have key people who support the current patch as well (jessebeach, msonnabaum, wimleers, moshe weitzman,..)
Comment #145
kattekrab CreditAttribution: kattekrab commentedWould a new issue summary help here?
Comment #146
kattekrab CreditAttribution: kattekrab commentedOh look! here's that garlic.js issue again.
Is anyone still interested in pursuing this? If yes, I'll happily refresh the issue summary to get everyone on the same page.
:)
Comment #147
cosmicdreams CreditAttribution: cosmicdreams commentedThis totally sounds like Drupal 8.3 material. If we can agree to develop and continue to improve Drupal past launch we can build support for this in later.
Comment #148
LewisNymanComment #149
kattekrab CreditAttribution: kattekrab commentedThis is a major pain point for content people. It's so so sooooo sad to see this can perpetually kicked down the road.
Someone said this could be 8.2 - and then someone says 8.3
And some say 9? Gah.
Comment #150
Bojhan CreditAttribution: Bojhan commented@Kattekrab No worries, I don't think anyone can predict that far away. If anything it signals, that people are aware of the amount of work involved. It could easily be picked up whenever.
Comment #151
Bojhan CreditAttribution: Bojhan commented@Kattekrab No worries, I don't think anyone can predict that far away. If anything it signals, that people are aware of the amount of work involved. It could easily be picked up whenever.
Comment #152
Bojhan CreditAttribution: Bojhan commented@Kattekrab No worries, I don't think anyone can predict that far away. If anything it signals, that people are aware of the amount of work involved. It could easily be picked up whenever.
Comment #153
David_Rothstein CreditAttribution: David_Rothstein commentedThe original issue here is definitely a bug. (Garlic.js was just one possible solution.)
I'd be interested to see testing with the latest Drupal codebase and with modern browsers to see where things currently stand (for examples from very long ago see #3 and #18 although more recently than that people have said it's Chrome too).
I think @casey's approach from #30 still looks interesting and could use some testing as well.
Preserving form data in other situations (e.g. after closing the browser) should really be a separate issue (and that sounds like a feature request) since it gets quite a bit more complicated.
Comment #155
Wim LeersThis was just reported again, this time against CKEditor: #2767979: Clicked "Back" then "forward" loses body text on new node creation.
Comment #157
laVera CreditAttribution: laVera commentedI came to this issue after loosing many hours of redaction work, and I have to say: WTF! Shame on all us!
I totally agree with @kattekrab and @Crell this is a big bug, and if is possible to fix it (even in a nasty way) it should be done ASAP.
But reading here how this issue was treated, I feel like last @dries keynote was an empty promise on content creators (Donald Trump level empty promise). Content people needs' are not only underrepresented, but also pushed down right here on issue tracker.
I never before felt so disappointed with our community; I don't mean to offend personally to anybody. But I do mean to call you all attention here, this issue need it.
So, my question to the seniors here:
How should a Drupaler next door —like me— should deal with this?
Comment #158
droplet CreditAttribution: droplet commentedSaving data into localStorage lead to another security issue. We could close/hang the browser at any time. Sensitive data we filled will be exposed.
I believed a simple popup warning message could save most of those posting here. And then seek for the perfect deal. Gmail-like app also popup a message before they actual saving draft into the remote server. It's almost a must element.
@nahuel,
This post has pretty a lot discussion. To help other issues, you can post your thoughts here: #2706483: [policy, no patch] Policy to help less interested Patch move forward.. Don't kick developers with passions out! Actually, I mentioned `What Is the Real Critical Issue` there: https://www.drupal.org/node/2706483#comment-11250037
Comment #159
Benia CreditAttribution: Benia commentedI suggest adding autosave system as they did a few years ago in Wordpress... It could improve much of the edit experience in Drupal and will align with the major improvements in Drupal 8.
Comment #160
Benia CreditAttribution: Benia commentedOpened a discussion about an autosave system here:
https://www.drupal.org/node/2801839#comment-11633647
Comment #164
margyly CreditAttribution: margyly commentedI love the idea of autosave and the users and my organization would love it, too. Sorry I can't help make it happen!
Comment #168
lauriiiComment #169
catchI just briefly tested this on chromium, on Drupal 7 (Drupal.org) and Drupal 9.
1. Entering content into a form, clicking off (in my case to admin/structure), then hitting the back button, doesn't lose data from regular form fields.
2. Doing the same, but with ckeditor4 in a textarea does.
This needs testing safari, edge, firefox to see if the behaviour is the same. We could also test the same thing with ckeditor5. If it's fixed in ckeditor 5 and consistent across browsers, I think we could close this issue (I don't see us successfully fixing it for ckeditor 4, and the work would be obsolete for Drupal 10).
Comment #170
pameeela CreditAttribution: pameeela commentedDiscussed with @catch. Since this only applies to CKEditor fields now, and the proposed fix was an autosave feature, I'm going to close this in favour of #3029488: Implement Autosave.