Moodle
  1. Moodle
  2. MDL-18014

Automatically and periodically save WYSIWYG editor content as a draft/concept

    Details

    • Testing Instructions:
      Hide

      Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

      Open a page with an Atto editor.
      Type some text - wait more than 60 seconds.
      Leave the page without submitting the form.
      Go back to the page.
      Verify a small notification shows in the editor and the unsaved text was recovered.

      With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
      Start apache again, verify the warning goes away after 60 seconds.
      With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.

      Load a forum - make 2 posts. Start replying to the first post, then wait until the "Draft saved" message is shown. Then leave the page without saving the form. Start replying to the second post and verify the draft is not recovered for the wrong post. Go back and start replying to the first post and ensure the draft is recovered because it matches this post.

      Another potentially tricky case:

      1. Create a quiz with an Essay question in it.
      2. Attempt the quiz as two different students.
      3. Go to Results -> Manual grading, and select the essay question.
      4. Verify that autosave works on the two editors on that page.
      Show
      Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window. Open a page with an Atto editor. Type some text - wait more than 60 seconds. Leave the page without submitting the form. Go back to the page. Verify a small notification shows in the editor and the unsaved text was recovered. With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.". Start apache again, verify the warning goes away after 60 seconds. With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning. Load a forum - make 2 posts. Start replying to the first post, then wait until the "Draft saved" message is shown. Then leave the page without saving the form. Start replying to the second post and verify the draft is not recovered for the wrong post. Go back and start replying to the first post and ensure the draft is recovered because it matches this post. Another potentially tricky case: Create a quiz with an Essay question in it. Attempt the quiz as two different students. Go to Results -> Manual grading, and select the essay question. Verify that autosave works on the two editors on that page.
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_28_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-18014-master
    • Story Points (Obsolete):
      40
    • Sprint:
      BACKEND Sprint 15

      Description

      This emerged during the discussion at MDL-13370. Shortly: use AJAX to automatically and periodically (eg. every minute) save the content of a WYSIWYG editor to the concepts (drafts) space. See GMail for the working example.

      It seems to me that we could use table mdl_post for this therefore not DB changes should be required. There will be a flag for a rich text field definitions that enables this feature. Every WYSIWYG will have to have some uniq identifier. Concepts are accessible to their owners only (and admins?) - maybe via a card in their profiles? Their lifetime is a subject of configuration - couple of hours by default.

        Gliffy Diagrams

        1. MDL-18014-autosave-alpha1.diff.txt
          19 kB
          David Mudrak
        1. autosave-1.png
          28 kB
        2. autosave-2.png
          29 kB
        3. autosave-3.png
          7 kB
        4. autosave-4.png
          9 kB
        5. notices.jpg
          18 kB
        6. OK and warning.png
          2 kB
        7. shot.png
          67 kB

          Issue Links

            Activity

            David Mudrak created issue -
            David Mudrak made changes -
            Field Original Value New Value
            Link This issue has been marked as being related by MDL-13370 [ MDL-13370 ]
            David Mudrak made changes -
            Link This issue will help resolve MDL-16732 [ MDL-16732 ]
            David Mudrak made changes -
            Link This issue will help resolve MDL-14963 [ MDL-14963 ]
            Hide
            David Mudrak added a comment -

            By Olli Savolainen at http://moodle.org/mod/forum/discuss.php?d=74710 :

            "A similar thing could be done, yes, for forum posting, too, or for that matter any other form field. Since Moodle uses YUI instead of prototype, it should be done with that, and probably we should try to make it more general than just something that works for quiz or for forum; i.e. this kind of functionality should be integrated in the editor itself and a general backend to receive the autosaves would need to be created."

            Show
            David Mudrak added a comment - By Olli Savolainen at http://moodle.org/mod/forum/discuss.php?d=74710 : "A similar thing could be done, yes, for forum posting, too, or for that matter any other form field. Since Moodle uses YUI instead of prototype, it should be done with that, and probably we should try to make it more general than just something that works for quiz or for forum; i.e. this kind of functionality should be integrated in the editor itself and a general backend to receive the autosaves would need to be created."
            David Mudrak made changes -
            Link This issue will help resolve MDL-15220 [ MDL-15220 ]
            David Mudrak made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            David Mudrak added a comment -

            I've got first working prototype of autosaving. At the moment it can support all form fields rendered by print_textarea(). There are several important aspects to be discussed:

            1. at the moment, I save the content of the field into the table 'post' where module is 'autosave'. Is it ok?
            2. how shall we implement a recovery procedure? i mean - what the user has to do to get the autosaved contents? I can see two possible approaches:
            2.1. add another tab into the user profile where she can find all her recent autosaved fields. This is easy to implement and might be enough given that autosave feature is just a backup&rescue tool
            2.2. after the page with a supported field is loaded, try to check if we have a previously autosaved content and offer it to user. This is more difficult to implement. How to make sure we are at the same page again? URL may have changed since the field was autosaved...

            Show
            David Mudrak added a comment - I've got first working prototype of autosaving. At the moment it can support all form fields rendered by print_textarea(). There are several important aspects to be discussed: 1. at the moment, I save the content of the field into the table 'post' where module is 'autosave'. Is it ok? 2. how shall we implement a recovery procedure? i mean - what the user has to do to get the autosaved contents? I can see two possible approaches: 2.1. add another tab into the user profile where she can find all her recent autosaved fields. This is easy to implement and might be enough given that autosave feature is just a backup&rescue tool 2.2. after the page with a supported field is loaded, try to check if we have a previously autosaved content and offer it to user. This is more difficult to implement. How to make sure we are at the same page again? URL may have changed since the field was autosaved...
            David Mudrak made changes -
            Component/s HTML Editor [ 10070 ]
            David Mudrak made changes -
            Attachment autosave-1.png [ 16135 ]
            David Mudrak made changes -
            Attachment autosave-2.png [ 16136 ]
            Hide
            Tim Hunt added a comment -

            This is a valuable feature, but ...

            1. I don't think that the post table is the right place for this. Make a new table specifically for this purpose.

            2. Recovery should be as transparent as possible.

            3. If this is worth doing, surely it is worth doing for all form fields.

            4. What does this do to server load. For example it is not much good auto-saving data during a quiz attempt, if the result is to overload the server and crash it when otherwise everything would be fine.

            5. What are the security implications. For example, if an malicious person can insert arbitrary content into the admin's autosaved fields area, then when the admin goes to look at that content, he is vulnerable to XSS vulnerabilities. Of course, the more validation you try to do, the more the performance implications.

            6. Whatever happens, we need an on/off switch in the admin settings, so admins can control this.

            7. What are we actually trying to achieve here anyway? Are we saving people from browser crashes, session problems, server crashes, what?

            Show
            Tim Hunt added a comment - This is a valuable feature, but ... 1. I don't think that the post table is the right place for this. Make a new table specifically for this purpose. 2. Recovery should be as transparent as possible. 3. If this is worth doing, surely it is worth doing for all form fields. 4. What does this do to server load. For example it is not much good auto-saving data during a quiz attempt, if the result is to overload the server and crash it when otherwise everything would be fine. 5. What are the security implications. For example, if an malicious person can insert arbitrary content into the admin's autosaved fields area, then when the admin goes to look at that content, he is vulnerable to XSS vulnerabilities. Of course, the more validation you try to do, the more the performance implications. 6. Whatever happens, we need an on/off switch in the admin settings, so admins can control this. 7. What are we actually trying to achieve here anyway? Are we saving people from browser crashes, session problems, server crashes, what?
            Hide
            David Mudrak added a comment -

            Thanks for your "buts", Tim.

            re 1. I have chosen the current post table because they are simply there and ready, which makes the development and testing much much easier (no need to upgrade, bump versions etc.). I will keep in mind that the physical storage can be change in the future. However, all I/O operation are wrapped so it won't be hard task to rewrite this part.

            re 2. well, it matters. I understand this feature as a rescue tool for users whose browsers do not cache the content of our HTML editor. Moreover, because TinyMCE does not update the content of the textarea it is linked to automatically but at the moment of saving the form (using tinyMCE.triggerSave(} call), the browser has nothing to cache. So, I do not think we really need a transparent recovery. Ask somebody who spent half an hour writing a forum post, then lost the focus of the editor (by a popup for example), returned back (after closing the popup) and pressed the backspace because of a typo. He is redirected to the previous page and after returning back, the HTML editor is empty. Such user will be more than happy if he is able to find a minute old draft of his post anywhere in the Moodle, without the need to get the content automatically. Of course, I would like to see a transparent and silent recovery more, but it makes the whole mechanism more complicated. I like things simply stupid.

            re 3. sure it is. Let us start with the support of HTML editor. Usually standard form fields are pretty well cached by the browser itself and the input is not so long.

            re 4 + 6. totally agree. Admins will have to enable this feature and set the frequency of the autosaving (influnce the server load) and the draft lifetime

            5. a priority issue, of course. That is why I wanted to put automatically saved drafts into a user space, ie. his profile. No automatic pre-filling of forms from drafts. If I loose something, I will find it in a rescue box at my profile page. Should standard sesskey() validation be enough in this case?

            7. see the users' stories in the linked issues. Basically this should protect from browser crashes, accidental redirections, window/tab close, connectivity problems, server crash etc. Actually I can't see the point of this question.

            Show
            David Mudrak added a comment - Thanks for your "buts", Tim. re 1. I have chosen the current post table because they are simply there and ready, which makes the development and testing much much easier (no need to upgrade, bump versions etc.). I will keep in mind that the physical storage can be change in the future. However, all I/O operation are wrapped so it won't be hard task to rewrite this part. re 2. well, it matters. I understand this feature as a rescue tool for users whose browsers do not cache the content of our HTML editor. Moreover, because TinyMCE does not update the content of the textarea it is linked to automatically but at the moment of saving the form (using tinyMCE.triggerSave(} call), the browser has nothing to cache. So, I do not think we really need a transparent recovery. Ask somebody who spent half an hour writing a forum post, then lost the focus of the editor (by a popup for example), returned back (after closing the popup) and pressed the backspace because of a typo. He is redirected to the previous page and after returning back, the HTML editor is empty. Such user will be more than happy if he is able to find a minute old draft of his post anywhere in the Moodle, without the need to get the content automatically. Of course, I would like to see a transparent and silent recovery more, but it makes the whole mechanism more complicated. I like things simply stupid. re 3. sure it is. Let us start with the support of HTML editor. Usually standard form fields are pretty well cached by the browser itself and the input is not so long. re 4 + 6. totally agree. Admins will have to enable this feature and set the frequency of the autosaving (influnce the server load) and the draft lifetime 5. a priority issue, of course. That is why I wanted to put automatically saved drafts into a user space, ie. his profile. No automatic pre-filling of forms from drafts. If I loose something, I will find it in a rescue box at my profile page. Should standard sesskey() validation be enough in this case? 7. see the users' stories in the linked issues. Basically this should protect from browser crashes, accidental redirections, window/tab close, connectivity problems, server crash etc. Actually I can't see the point of this question.
            Hide
            David Mudrak added a comment -

            Well. After submitting the previous post I repeated some tests to be sure and realized that maybe my arguments are useless. TinyMCE in HEAD is cached in my FF3 when navigating to the previous page and back again :-/ It does not work with the current editor (HTMLArea is it?) in 1.9 stable but does in HEAD. Hmm... much ado about nothing? I will test IE and Opera yet. Can some Apple fan try Safari please?

            Show
            David Mudrak added a comment - Well. After submitting the previous post I repeated some tests to be sure and realized that maybe my arguments are useless. TinyMCE in HEAD is cached in my FF3 when navigating to the previous page and back again :-/ It does not work with the current editor (HTMLArea is it?) in 1.9 stable but does in HEAD. Hmm... much ado about nothing? I will test IE and Opera yet. Can some Apple fan try Safari please?
            Hide
            Matt Gibson added a comment -

            Regarding point 7, there is also simply forgetting about a tab you've been working on and shutting the browser. I've done this three times in the last two weeks whilst writing long schemes of work into a wiki and it's driving me nuts.

            It occurred to me that it may not be obvious to people that they have a saved draft at all (new feature may not be widely publicised by admins), which would be a real waste of a great feature, so it might be handy if there was some visual indicator e.g. an 'unsaved work' icon next to the item on the main course page and also within the activity itself.

            Show
            Matt Gibson added a comment - Regarding point 7, there is also simply forgetting about a tab you've been working on and shutting the browser. I've done this three times in the last two weeks whilst writing long schemes of work into a wiki and it's driving me nuts. It occurred to me that it may not be obvious to people that they have a saved draft at all (new feature may not be widely publicised by admins), which would be a real waste of a great feature, so it might be handy if there was some visual indicator e.g. an 'unsaved work' icon next to the item on the main course page and also within the activity itself.
            Hide
            Mark Nielsen added a comment -

            What about implementing this through the Moodle form API? The form could have a unique ID (perhaps just use the form's class name?), which is saved with its data into the database. If the form is loaded and there is info for that user in the db, then automatically load the form with the data or ask the user if s/he would like to load it with the data. On submit, the data in the database gets cleared. This route would be transparent for users and turn the form API into an even more powerful tool for developers.

            I would also suggest using the YUI since it is part of Moodle's codebase and it is relatively simple to have it submit a form's data to a URL. Also, it has a nice event listener API that's very helpful for this sort of work.

            A note on performance, one way to help reduce the number of saves is not just by doing it periodically, but also not saving while the user is typing because as soon as you save, it is out of date anyways. You can accomplish this with timeouts by setting one when you detect a change in the form, and then if you detect another change before the timeout has come to pass, then clear/cancel the old timeout and set a new one.

            Show
            Mark Nielsen added a comment - What about implementing this through the Moodle form API? The form could have a unique ID (perhaps just use the form's class name?), which is saved with its data into the database. If the form is loaded and there is info for that user in the db, then automatically load the form with the data or ask the user if s/he would like to load it with the data. On submit, the data in the database gets cleared. This route would be transparent for users and turn the form API into an even more powerful tool for developers. I would also suggest using the YUI since it is part of Moodle's codebase and it is relatively simple to have it submit a form's data to a URL. Also, it has a nice event listener API that's very helpful for this sort of work. A note on performance, one way to help reduce the number of saves is not just by doing it periodically, but also not saving while the user is typing because as soon as you save, it is out of date anyways. You can accomplish this with timeouts by setting one when you detect a change in the form, and then if you detect another change before the timeout has come to pass, then clear/cancel the old timeout and set a new one.
            Hide
            David Mudrak added a comment -

            IMHO the form's class is not enough here - the autosave ID should combine the field id and the URL. But in theory the URL may change while still pointing to the same page where the draft was saved.

            Yes, the current implementation is based on YUI.

            Hmm, I don't think it is good to wait for the user stop typing. It means that the work of "quick-typers" is never autosaved. Instead, there is no need to send an update call to server in case of no change. We could do some statistics based on moodle.org logs: how long does it take to write an average forum post? Ie. the time from displaying the form to the submitting.

            Show
            David Mudrak added a comment - IMHO the form's class is not enough here - the autosave ID should combine the field id and the URL. But in theory the URL may change while still pointing to the same page where the draft was saved. Yes, the current implementation is based on YUI. Hmm, I don't think it is good to wait for the user stop typing. It means that the work of "quick-typers" is never autosaved. Instead, there is no need to send an update call to server in case of no change. We could do some statistics based on moodle.org logs: how long does it take to write an average forum post? Ie. the time from displaying the form to the submitting.
            Hide
            Mark Nielsen added a comment -

            "IMHO the form's class is not enough here - the autosave ID should combine the field id and the URL. But in theory the URL may change while still pointing to the same page where the draft was saved."

            What about using $CFG->pagepath? On the other hand, I wasn't necessarily thinking that we would just enable autosave automatically for every form in Moodle, might be better to have devs implement in the necessary areas. In which case, the dev could assign an appropriate ID. Just another though.

            Show
            Mark Nielsen added a comment - "IMHO the form's class is not enough here - the autosave ID should combine the field id and the URL. But in theory the URL may change while still pointing to the same page where the draft was saved." What about using $CFG->pagepath? On the other hand, I wasn't necessarily thinking that we would just enable autosave automatically for every form in Moodle, might be better to have devs implement in the necessary areas. In which case, the dev could assign an appropriate ID. Just another though.
            Hide
            David Mudrak added a comment -

            Yes, that might be a solution. Of course that fields that should be autosaved will be manually flaged by a developer. I was thinking of forum post bodies only at this moment and there should be no problem identifying such fields.

            Show
            David Mudrak added a comment - Yes, that might be a solution. Of course that fields that should be autosaved will be manually flaged by a developer. I was thinking of forum post bodies only at this moment and there should be no problem identifying such fields.
            Hide
            Sam Marshall added a comment -

            Re screenshot:

            1 'Saving' should prob. be 'Saving draft' or even 'Autosaving draft'. Don't want to scare users into thinking it's already been submitted.

            2 The saving text should be much smaller

            Show
            Sam Marshall added a comment - Re screenshot: 1 'Saving' should prob. be 'Saving draft' or even 'Autosaving draft'. Don't want to scare users into thinking it's already been submitted. 2 The saving text should be much smaller
            Hide
            David Mudrak added a comment -

            I like sam's idea (from the jabber chatroom):

            (15:29:09) sam: can it store the page id (mod-forum-editpost or whatever) and then when you go to that page again (any forum post), display a little '1 recent autosaved draft is available' thing that you can click on to see it and/or get it back somehow?

            Show
            David Mudrak added a comment - I like sam's idea (from the jabber chatroom): (15:29:09) sam: can it store the page id (mod-forum-editpost or whatever) and then when you go to that page again (any forum post), display a little '1 recent autosaved draft is available' thing that you can click on to see it and/or get it back somehow?
            Hide
            David Mudrak added a comment -

            More screenshots to demonstrate the current implementation:

            1. after the page is loaded, an AJAX based check is performed to get know if there is some previously autosaved draft
            2. if no saved draft is found, the autosaving feature is enabled and the field content is saved every xxx seconds
            3. if there is a draft found, an information appears (with a "yellow fade efect" to draw user's attention) and the draft can be restored with a single click

            Show
            David Mudrak added a comment - More screenshots to demonstrate the current implementation: 1. after the page is loaded, an AJAX based check is performed to get know if there is some previously autosaved draft 2. if no saved draft is found, the autosaving feature is enabled and the field content is saved every xxx seconds 3. if there is a draft found, an information appears (with a "yellow fade efect" to draw user's attention) and the draft can be restored with a single click
            David Mudrak made changes -
            Attachment autosave-3.png [ 16183 ]
            Attachment autosave-4.png [ 16184 ]
            Hide
            David Mudrak added a comment -

            The actual state of my patch has been attached as autosave-alpha1.diff.txt. Any comments/review/suggestions etc. warmly welcome

            Show
            David Mudrak added a comment - The actual state of my patch has been attached as autosave-alpha1.diff.txt. Any comments/review/suggestions etc. warmly welcome
            David Mudrak made changes -
            Attachment MDL-18014-autosave-alpha1.diff.txt [ 16206 ]
            David Mudrak made changes -
            Status In Progress [ 3 ] Open [ 1 ]
            Hide
            David Mudrak added a comment -

            Stopping progress on this for a while. Petr informed me there are some planned changes in the HEAD leading to introduce a new form element. The element will integrate wysiwyg editor, image uploader and attachment uploader. It looks reasonable for me to wait for this component to be included and than support autosave of it.

            Show
            David Mudrak added a comment - Stopping progress on this for a while. Petr informed me there are some planned changes in the HEAD leading to introduce a new form element. The element will integrate wysiwyg editor, image uploader and attachment uploader. It looks reasonable for me to wait for this component to be included and than support autosave of it.
            David Mudrak made changes -
            Priority Major [ 3 ] Minor [ 4 ]
            Minh-Tam Nguyen made changes -
            Link This issue will help resolve MDL-15855 [ MDL-15855 ]
            Hide
            Mauno Korpelainen added a comment -

            David - can you check http://moodle.org/mod/forum/discuss.php?d=130005&parent=569051#p569547 and http://code.google.com/p/tinyautosave/

            TinyAutoSave plug-in works very well in moodle 2.0 (for textareas of tinymce) as an additional / optional autosave feature (should be possible to enable/disable from administration of course)

            Show
            Mauno Korpelainen added a comment - David - can you check http://moodle.org/mod/forum/discuss.php?d=130005&parent=569051#p569547 and http://code.google.com/p/tinyautosave/ TinyAutoSave plug-in works very well in moodle 2.0 (for textareas of tinymce) as an additional / optional autosave feature (should be possible to enable/disable from administration of course)
            Hide
            Martin Dougiamas added a comment -

            I would still very like to see this in 2.0 if it's not going to be very hard to do.

            Show
            Martin Dougiamas added a comment - I would still very like to see this in 2.0 if it's not going to be very hard to do.
            Martin Dougiamas made changes -
            Priority Minor [ 4 ] Major [ 3 ]
            Hide
            David Mudrak added a comment -

            The purpose of TinyAutoSave is just to prevent from accidental navigating away from the page (by clicking a link or pressing Backspace button). The disadvantage is that it does not store the draft on the server so it is not the same concept as discussed above. But the advantage is that it does not store the draft on the server so there are no performance/bandwidth issues.

            This issue requires some decisions on the required functionality so that we decide about the target version.

            1. Should the embedded media and attached files be autosaved, too? I am not sure our File API can support this at the moment. What file area would we used? What would be the context of the files? And itemids?
            2. Should the draft be saved on server side or client side?

            Show
            David Mudrak added a comment - The purpose of TinyAutoSave is just to prevent from accidental navigating away from the page (by clicking a link or pressing Backspace button). The disadvantage is that it does not store the draft on the server so it is not the same concept as discussed above. But the advantage is that it does not store the draft on the server so there are no performance/bandwidth issues. This issue requires some decisions on the required functionality so that we decide about the target version. 1. Should the embedded media and attached files be autosaved, too? I am not sure our File API can support this at the moment. What file area would we used? What would be the context of the files? And itemids? 2. Should the draft be saved on server side or client side?
            Hide
            Minh-Tam Nguyen added a comment -

            I think that saving the draft on the server is a better solution, as this will protect the draft from getting lost if the user's computer crashes.

            A case that is occasionally reported to us here is that students start typing their post, but then for whatever reason their computer crashes.
            This means that they have to log on again, potentially on a different computer, and in our particular case on a computer that has been deep-frozen. As such, any cached drafts will be lost.

            I think that if the draft was saved on the server, the user could then pick up where they left no matter if they switched computer or not.

            Show
            Minh-Tam Nguyen added a comment - I think that saving the draft on the server is a better solution, as this will protect the draft from getting lost if the user's computer crashes. A case that is occasionally reported to us here is that students start typing their post, but then for whatever reason their computer crashes. This means that they have to log on again, potentially on a different computer, and in our particular case on a computer that has been deep-frozen. As such, any cached drafts will be lost. I think that if the draft was saved on the server, the user could then pick up where they left no matter if they switched computer or not.
            Hide
            Koen Roggemans added a comment -

            It is a difficult question. A network connection failure at saving time will probably cause the text and the draft to be lost.
            I guess saving it simultaneously local and server side is not an option ?

            Show
            Koen Roggemans added a comment - It is a difficult question. A network connection failure at saving time will probably cause the text and the draft to be lost. I guess saving it simultaneously local and server side is not an option ?
            Hide
            Mauno Korpelainen added a comment -

            Server side and/or client side solutions could be used at the same time - current core autosave plugin of tinymce has supported local storage methods since January (no more need to download separate TinyAutoSave plugin anymore) - moodle 2.0 should just be upgraded to use the latest version of tinymce (3.3.6) and moodle sites can add/enable client side autosave feature in init code for those textareas that use tinymce with possible functionality settings http://code.google.com/p/tinyautosave/wiki/Functionality . Autosave plugin supports all the browsers that tinymce supports.

            The primary solution for temporary autosaving user input data might still be a server side solution as far as it can be used and we might need a setting to administration to be able to enable/disable either one or both of these options. In my opinion it does not need to be too complex (at first), the most common failure is that users accidentally press some link or button that moves them away from editor screen or they press some button that closes window/moodle. HTMLArea has failed to keep input data in such cases, tinymce with autosave plugin can in most cases restore the last data.

            Keeping track of user processes might be usefull for other purposes as well - for example Version control here in tracker offers a nice way to track back last changes in files and in Wiki it is useful to find the last changes saved by different users. The main difficulty in autosaving data is in my opinion the choice of deleting unnecessary data - particularly duplicate files - as soon as possible

            So if we need to autosave also attachments (clone attachments) clones should be deleted immediatelly after the actual data & attachments have been saved (or small hosted sites run easily out of hard disc space

            Show
            Mauno Korpelainen added a comment - Server side and/or client side solutions could be used at the same time - current core autosave plugin of tinymce has supported local storage methods since January (no more need to download separate TinyAutoSave plugin anymore) - moodle 2.0 should just be upgraded to use the latest version of tinymce (3.3.6) and moodle sites can add/enable client side autosave feature in init code for those textareas that use tinymce with possible functionality settings http://code.google.com/p/tinyautosave/wiki/Functionality . Autosave plugin supports all the browsers that tinymce supports. The primary solution for temporary autosaving user input data might still be a server side solution as far as it can be used and we might need a setting to administration to be able to enable/disable either one or both of these options. In my opinion it does not need to be too complex (at first), the most common failure is that users accidentally press some link or button that moves them away from editor screen or they press some button that closes window/moodle. HTMLArea has failed to keep input data in such cases, tinymce with autosave plugin can in most cases restore the last data. Keeping track of user processes might be usefull for other purposes as well - for example Version control here in tracker offers a nice way to track back last changes in files and in Wiki it is useful to find the last changes saved by different users. The main difficulty in autosaving data is in my opinion the choice of deleting unnecessary data - particularly duplicate files - as soon as possible So if we need to autosave also attachments (clone attachments) clones should be deleted immediatelly after the actual data & attachments have been saved (or small hosted sites run easily out of hard disc space
            Martin Dougiamas made changes -
            Fix Version/s 2.0.1 [ 10420 ]
            Fix Version/s 2.0 [ 10122 ]
            Martin Dougiamas made changes -
            Workflow jira [ 30402 ] MDL Workflow [ 44403 ]
            Martin Dougiamas made changes -
            Fix Version/s 2.0.2 [ 10421 ]
            Fix Version/s 2.0.1 [ 10420 ]
            Martin Dougiamas made changes -
            Fix Version/s 2.0.3 [ 10537 ]
            Fix Version/s 2.0.2 [ 10421 ]
            David Mudrak made changes -
            Fix Version/s DEV backlog [ 10464 ]
            Fix Version/s 2.0.3 [ 10537 ]
            Hide
            David Mudrak added a comment -

            Re-triage: this must be re-considered for some future version, not in stable.

            Show
            David Mudrak added a comment - Re-triage: this must be re-considered for some future version, not in stable.
            Martin Dougiamas made changes -
            Workflow MDL Workflow [ 44403 ] MDL Full Workflow [ 72778 ]
            Hide
            Koen Roggemans added a comment -

            Please bump this feature request to a near Moodle release. A lot of Web 2.0 applications support auto-save by now, Moodle can't stay behind on this. This request crops up every now and then on the work floor.

            Show
            Koen Roggemans added a comment - Please bump this feature request to a near Moodle release. A lot of Web 2.0 applications support auto-save by now, Moodle can't stay behind on this. This request crops up every now and then on the work floor.
            Hide
            Dimuthu Upeksha added a comment -

            I am new for moodle development and hope to participate gsoc 2012. I think we can use AJAX support to implement this feature. Autosaving is done in background and we can specify the way it should be saved (auto save per minute or when some events are triggered by html elements). I think it is easy if we can add new a new table to store those auto saved items. In that table we can specify user id, last edited time, edited url, data.
            In data field, we can store values of those elements as key value pairs
            eg: (element1 Id: value),(element2 Id: value) likewise
            From url we can identify correct position.

            We can extend this structure to even forms in the applications.
            Those auto saved values are shown in different color so that user can identify it.

            Any ideas?

            Show
            Dimuthu Upeksha added a comment - I am new for moodle development and hope to participate gsoc 2012. I think we can use AJAX support to implement this feature. Autosaving is done in background and we can specify the way it should be saved (auto save per minute or when some events are triggered by html elements). I think it is easy if we can add new a new table to store those auto saved items. In that table we can specify user id, last edited time, edited url, data. In data field, we can store values of those elements as key value pairs eg: (element1 Id: value),(element2 Id: value) likewise From url we can identify correct position. We can extend this structure to even forms in the applications. Those auto saved values are shown in different color so that user can identify it. Any ideas?
            Hide
            David Mudrak added a comment -

            Dimuthu, thanks for considering this for the GSoC project. The HTML text contents itself is not a big issue. Earlier in the 2.0 development branch (before TinyMCE and the new Files API were integrated), I already had a working prototype for that. But the real issue here are files - embedded media and attachments. While editing a form, they are stored in a temporary draft file area and we need to come with a solution, where and how store these files during the lifetime of the form draft/concept. This is the real challenge of this project. It requires deep understanding of the Files API and TinyMCE integration in Moodle. The rest is just fun with YUI and AJAX

            Show
            David Mudrak added a comment - Dimuthu, thanks for considering this for the GSoC project. The HTML text contents itself is not a big issue. Earlier in the 2.0 development branch (before TinyMCE and the new Files API were integrated), I already had a working prototype for that. But the real issue here are files - embedded media and attachments. While editing a form, they are stored in a temporary draft file area and we need to come with a solution, where and how store these files during the lifetime of the form draft/concept. This is the real challenge of this project. It requires deep understanding of the Files API and TinyMCE integration in Moodle. The rest is just fun with YUI and AJAX
            Hide
            Dimuthu Upeksha added a comment -

            David, thank you for reply In the case of uploading attachments and media files I think major issue is having enough server space for that drafts. Now I'm looking at the areas you have explained. However I have some ideas about managing those files.
            1. Allowing maximum draft file storage size for each user (about 20 MB). If it is exceeded old files are deleted and gives chance to new files
            2. If the draft is not reused for some period those drafts are automatically deleted.
            3. Using external server space to store those files. It will reduce the external load on moodle server.

            Show
            Dimuthu Upeksha added a comment - David, thank you for reply In the case of uploading attachments and media files I think major issue is having enough server space for that drafts. Now I'm looking at the areas you have explained. However I have some ideas about managing those files. 1. Allowing maximum draft file storage size for each user (about 20 MB). If it is exceeded old files are deleted and gives chance to new files 2. If the draft is not reused for some period those drafts are automatically deleted. 3. Using external server space to store those files. It will reduce the external load on moodle server.
            Hide
            Josh Johnston added a comment -

            What about skipping the server side storage and using a prebuilt html 5 localStorage API like http://garlicjs.org/ ?

            Show
            Josh Johnston added a comment - What about skipping the server side storage and using a prebuilt html 5 localStorage API like http://garlicjs.org/ ?
            Michael de Raadt made changes -
            Labels triaged
            Affects Version/s 2.4.1 [ 12461 ]
            Michael de Raadt made changes -
            Link This issue will help resolve MDL-37980 [ MDL-37980 ]
            Hide
            Rex Lorenzo added a comment -

            Bumping this issue as a possible UI improvement for the FRONTEND team to focus on for Moodle 2.6. Lots of different ideas and possible solutions, but anything would be an improvement.

            Although, a downside to using HTML local storage is that the user has to hit a confirmation approval before a website can store anything in their browser. Which is potentially very annoying and can cause confusion if they say "No", but later want to enable it.

            Show
            Rex Lorenzo added a comment - Bumping this issue as a possible UI improvement for the FRONTEND team to focus on for Moodle 2.6. Lots of different ideas and possible solutions, but anything would be an improvement. Although, a downside to using HTML local storage is that the user has to hit a confirmation approval before a website can store anything in their browser. Which is potentially very annoying and can cause confusion if they say "No", but later want to enable it.
            David Mudrak made changes -
            Assignee David Mudrak [ mudrd8mz ] moodle.com [ moodle.com ]
            David Mudrak made changes -
            Fix Version/s FRONTEND [ 12581 ]
            Martin Dougiamas made changes -
            Rank Ranked lower
            Hide
            Stuart Lamour added a comment -

            Mark had this working a while back https://moodle.org/mod/forum/discuss.php?d=191050

            Show
            Stuart Lamour added a comment - Mark had this working a while back https://moodle.org/mod/forum/discuss.php?d=191050
            Martin Dougiamas made changes -
            Rank Ranked higher
            Hide
            Martin Dougiamas added a comment -

            I'll make sure this gets included in the "Editor upgrade project" for FRONTEND in 2.7.

            Show
            Martin Dougiamas added a comment - I'll make sure this gets included in the "Editor upgrade project" for FRONTEND in 2.7.
            Rex Lorenzo made changes -
            Link This issue has been marked as being related by MDL-38537 [ MDL-38537 ]
            Damyon Wiese made changes -
            Link This issue will be (partly) resolved by MDL-43434 [ MDL-43434 ]
            Martin Dougiamas made changes -
            Rank Ranked higher
            Hide
            Martin Dougiamas added a comment - - edited

            I like the idea of keeping it simple and using client-side storage. Even if (worst case) it was without the embedded files support initially, it would still be very useful to people.

            Show
            Martin Dougiamas added a comment - - edited I like the idea of keeping it simple and using client-side storage. Even if (worst case) it was without the embedded files support initially, it would still be very useful to people.
            Damyon Wiese made changes -
            Epic Link MDL-43841 [ 73298 ]
            Damyon Wiese made changes -
            Component/s HTML Editor (Atto) [ 13230 ]
            Component/s HTML Editor (TinyMCE) [ 10070 ]
            Component/s Forms Library [ 10091 ]
            Martin Dougiamas made changes -
            Assignee moodle.com [ moodle.com ] Andrew Nicols [ dobedobedoh ]
            Hide
            Andrew Nicols added a comment -

            I've created a proof-of-concept atto plugin for this. It's still not complete and I'll work more on it after the 2.7 release.
            The plugin can be found at https://github.com/andrewnicols/moodle-atto_storage

            Note: It currently does not deal with embedded files at all.

            Show
            Andrew Nicols added a comment - I've created a proof-of-concept atto plugin for this. It's still not complete and I'll work more on it after the 2.7 release. The plugin can be found at https://github.com/andrewnicols/moodle-atto_storage Note: It currently does not deal with embedded files at all.
            Hide
            Dan Poltawski added a comment -

            Looking at that POC it seems the the privacy of the local storage is not considered.

            From a philosophical point of view I think the 'browser session' should be protected by the user. From a practical point of view Moodle is often used in a shared-browser environment so we need to deal with this. One solution of the top of my head is that we could encrypt the data with a secret stored as a user preference.

            Show
            Dan Poltawski added a comment - Looking at that POC it seems the the privacy of the local storage is not considered. From a philosophical point of view I think the 'browser session' should be protected by the user. From a practical point of view Moodle is often used in a shared-browser environment so we need to deal with this. One solution of the top of my head is that we could encrypt the data with a secret stored as a user preference.
            Hide
            Andrew Nicols added a comment -

            Yup - I'm aware of that limitation at present and it's one I've been considering how best to plug. The problem is that there is no native cryptographic encryption in JavaScript, and even if there was it would inherently unsafe (see http://www.matasano.com/articles/javascript-cryptography/ for a pretty good summary).

            In reality someone would have to be actively and maliciously trying to steal data from the store, and have the key (fairly easy to get if we're talking non-SSL), or manipulate the page in some way in order to get the key or data (again, non-SSL or a direct browser attack). However, if someone can access the key in this way, then any way in which we store the data (e.g. AJAX queries to store in the DB) are also probably affected, and we will generally lose against a direct browser attack (that's to say a browser plugin hijacking data for example) so we would be no worse off.

            There is one main user-space Crypto suite - The Stanford Javascript Crypto Library (SJCL - https://github.com/bitwiseshiftleft/sjcl) but I need to look at how best to include it. It is compiled with ES6-style exports so I should be able to convert it into an ES6 YUI module now that we're on 3.15.0, but that's still beta code and pretty new. There is another way that I can include it, but I haven't had a chance to try implementing it yet.

            But rest assured, this is something I intend to address though before I create an official release of the plugin.

            Show
            Andrew Nicols added a comment - Yup - I'm aware of that limitation at present and it's one I've been considering how best to plug. The problem is that there is no native cryptographic encryption in JavaScript, and even if there was it would inherently unsafe (see http://www.matasano.com/articles/javascript-cryptography/ for a pretty good summary). In reality someone would have to be actively and maliciously trying to steal data from the store, and have the key (fairly easy to get if we're talking non-SSL), or manipulate the page in some way in order to get the key or data (again, non-SSL or a direct browser attack). However, if someone can access the key in this way, then any way in which we store the data (e.g. AJAX queries to store in the DB) are also probably affected, and we will generally lose against a direct browser attack (that's to say a browser plugin hijacking data for example) so we would be no worse off. There is one main user-space Crypto suite - The Stanford Javascript Crypto Library (SJCL - https://github.com/bitwiseshiftleft/sjcl ) but I need to look at how best to include it. It is compiled with ES6-style exports so I should be able to convert it into an ES6 YUI module now that we're on 3.15.0, but that's still beta code and pretty new. There is another way that I can include it, but I haven't had a chance to try implementing it yet. But rest assured, this is something I intend to address though before I create an official release of the plugin.
            Hide
            Dan Poltawski added a comment -

            Lets be clear about the 'attack scenario' I am referring to, because it is confusing the issue by talking about SSL and the security of ajax data transmission.

            1. Teacher 'Bob' uses shared browser in a university library to make a correction to their upcoming quiz questions
            2. Student 'Dan' sees the teacher on that workstation and uses it after the teacher
            3. Student 'Dan' uses the localstorage inspector to collect next weeks quiz questions before other students
            Show
            Dan Poltawski added a comment - Lets be clear about the 'attack scenario' I am referring to, because it is confusing the issue by talking about SSL and the security of ajax data transmission. Teacher 'Bob' uses shared browser in a university library to make a correction to their upcoming quiz questions Student 'Dan' sees the teacher on that workstation and uses it after the teacher Student 'Dan' uses the localstorage inspector to collect next weeks quiz questions before other students
            Hide
            Andrew Nicols added a comment -

            Yes - that's my understanding too. The attack comes in two forms:

            • someone has installed a browser plugin to inject additional code, or retrieve the key - this is a problem regardless and affects everything anyway so let's ignore it;
            • someone on the network sniffs for the key or manipulates the HTML during transmission to add additional code - again, this is already a concern, but only for sites not running SSL.

            The real difference here comes that we're now talking about data which may not be transferred over the wire so an attacker could sniff for the key, then see what content was left partially written.

            Mostly it's not a concern though because the attacker would just be getting access to data they already have access to.

            I should point out that on form submission, it nullifies the storage anyway so unless Bob makes his changes, then closes the browser without saving the changes, they won't be there anyway.

            Show
            Andrew Nicols added a comment - Yes - that's my understanding too. The attack comes in two forms: someone has installed a browser plugin to inject additional code, or retrieve the key - this is a problem regardless and affects everything anyway so let's ignore it; someone on the network sniffs for the key or manipulates the HTML during transmission to add additional code - again, this is already a concern, but only for sites not running SSL. The real difference here comes that we're now talking about data which may not be transferred over the wire so an attacker could sniff for the key, then see what content was left partially written. Mostly it's not a concern though because the attacker would just be getting access to data they already have access to. I should point out that on form submission, it nullifies the storage anyway so unless Bob makes his changes, then closes the browser without saving the changes, they won't be there anyway.
            Hide
            Andrew Nicols added a comment -

            Also your attack assumes that they're using the same browser, on the same user profile but this is of course a possibility.
            Either way, I do intend to address this wth the SCJL library.

            Show
            Andrew Nicols added a comment - Also your attack assumes that they're using the same browser, on the same user profile but this is of course a possibility. Either way, I do intend to address this wth the SCJL library.
            moodle.com made changes -
            Story Points 40
            moodle.com made changes -
            Sprint FRONTEND Sprint 12 [ 24 ]
            moodle.com made changes -
            Rank Ranked lower
            Hide
            Andrew Nicols added a comment -

            Unassigning this issue from myself in case someone else on the sprint wishes to work on it before I get the chance.

            Show
            Andrew Nicols added a comment - Unassigning this issue from myself in case someone else on the sprint wishes to work on it before I get the chance.
            Andrew Nicols made changes -
            Assignee Andrew Nicols [ dobedobedoh ]
            Hide
            Frédéric Massart added a comment -

            Commenting about my offline thoughts and discussions.

            It seems to me that before we go down the track of finding a solution, we should make sure we are all on the same page and understand what problems we are trying to solve. For instance, which ones of those user stories are we facing? (Maybe all of them)

            Ann is the teacher of Math 101. She is currently writing a rich document using the page module. Ann is clumsy and by mistake she hits CTRL + W, and confirms that she wants to leave the page. She is horrified seeing what she has just done. But as it gets late, she decides to go home. As she gets home, she logs back into Moodle with intention of starting her document from scratch. She creates a new page resource, and as the page loads a prompt asks her if she wants to reload the draft that she was working on. She clicks 'Yes' and her previous work is restored, including her images. She finishes the document and makes it available to students.

            Eric is a student of Math 101 and is submitting an online text assignment. As he finishes writing the assignment, he clicks "Save" but he appears to be logged out of Moodle. He logs back in and navigates to the assignment submission form. His unsent assignment is visible there and he simply clicks "Save" to really submit his assignment this time.

            From a public library Daniel is participating in a forum of Math 101, but as he was answering a discussion he realises that he has to run. His post not being ready yet, he uses a draft feature to save the content of his post and retrieve it later. When logging into Moodle from home he reloads his draft, finishes his post and submit it to the forum.

            Mary is using Moodle to write a blog post about her Math 101 course. So far she has written a few hundreds words, added images to her blog post, and is fairly confident that it is ready to be published. Unfortunately a power cut occurs and her computers goes down. When the power comes back, she now uses her iPad to log back into Moodle and is happy to find out that her content was somehow saved. She publishes her blog post instantly.

            Depending on which ones we pick, we will have to handle:

            • Drafts saved cross browsers
            • Drafts saved in case of session expiry
            • Drafts saved per user request
            • Drafts saved when connection is down

            Which have different implications:

            • The privacy of a draft content stored offline
            • The load on the server when sending the content back and forth
            • Handling diffs of content to save bandwidth

            We will also have to put thoughts in:

            • Clearly identifying the page/form/field on which the user is inputting content (the URL + form ID + element ID is not always enough)
            • Making a solution that works across editors
            Show
            Frédéric Massart added a comment - Commenting about my offline thoughts and discussions. It seems to me that before we go down the track of finding a solution, we should make sure we are all on the same page and understand what problems we are trying to solve. For instance, which ones of those user stories are we facing? (Maybe all of them) Ann is the teacher of Math 101. She is currently writing a rich document using the page module. Ann is clumsy and by mistake she hits CTRL + W, and confirms that she wants to leave the page. She is horrified seeing what she has just done. But as it gets late, she decides to go home. As she gets home, she logs back into Moodle with intention of starting her document from scratch. She creates a new page resource, and as the page loads a prompt asks her if she wants to reload the draft that she was working on. She clicks 'Yes' and her previous work is restored, including her images. She finishes the document and makes it available to students. Eric is a student of Math 101 and is submitting an online text assignment. As he finishes writing the assignment, he clicks "Save" but he appears to be logged out of Moodle. He logs back in and navigates to the assignment submission form. His unsent assignment is visible there and he simply clicks "Save" to really submit his assignment this time. From a public library Daniel is participating in a forum of Math 101, but as he was answering a discussion he realises that he has to run. His post not being ready yet, he uses a draft feature to save the content of his post and retrieve it later. When logging into Moodle from home he reloads his draft, finishes his post and submit it to the forum. Mary is using Moodle to write a blog post about her Math 101 course. So far she has written a few hundreds words, added images to her blog post, and is fairly confident that it is ready to be published. Unfortunately a power cut occurs and her computers goes down. When the power comes back, she now uses her iPad to log back into Moodle and is happy to find out that her content was somehow saved. She publishes her blog post instantly. Depending on which ones we pick, we will have to handle: Drafts saved cross browsers Drafts saved in case of session expiry Drafts saved per user request Drafts saved when connection is down Which have different implications: The privacy of a draft content stored offline The load on the server when sending the content back and forth Handling diffs of content to save bandwidth We will also have to put thoughts in: Clearly identifying the page/form/field on which the user is inputting content (the URL + form ID + element ID is not always enough) Making a solution that works across editors
            Hide
            Koen Roggemans added a comment -

            All stories are nice features to have, but if you need to prioritize, I think Mary and Eric are the most important once to fix: loosing work because of technical problems is a terrible user experience.
            The Mary story are two use cases in one: she could redo her post on the same computer or on a different one when the power comes back (to store locally or not to store locally).

            Show
            Koen Roggemans added a comment - All stories are nice features to have, but if you need to prioritize, I think Mary and Eric are the most important once to fix: loosing work because of technical problems is a terrible user experience. The Mary story are two use cases in one: she could redo her post on the same computer or on a different one when the power comes back (to store locally or not to store locally).
            Michael de Raadt made changes -
            Sprint FRONTEND Sprint 12 [ 24 ] BACKEND Sprint 13 [ 25 ]
            Michael de Raadt made changes -
            Rank Ranked lower
            Michael de Raadt made changes -
            Sprint BACKEND Sprint 13 [ 25 ] FRONTEND Sprint 13 [ 26 ]
            Michael de Raadt made changes -
            Rank Ranked higher
            Hide
            Dan Poltawski added a comment - - edited

            Great question Fred, I agree with Koen's comment.

            Some reflections based on your technical prompts:

            The privacy of a draft content stored offline

            It strikes me that for the sake of privacy, the best thing we can do is only use local storage in exceptional circumstances - when we can't save a draft to the server due to connection/session problems.

            The load on the server when sending the content back and forth
            Handling diffs of content to save bandwidth

            My advice would be to avoid premature optimisation with both of these.

            The extra load on the database is worth considering. But In short, do a small number of limited database queries and let the database server do what we pay it to do. It should be possible for most setups to deal with this comfortably and from what we know, they are doing with quiz-autosave which has the very worst of concurrency. Where we get into major problems with Moodle performance is when we start doing 100s of queries per page, or complex expensive queries. This isn't one of these cases.

            I think diffing of text content is a misnomer when you consider the relative size of one image compared to gzip'd text content, I wouldn't even consider it.

            Clearly identifying the page/form/field on which the user is inputting content (the URL + form ID + element ID is not always enough)

            Yup. Agreed, this is very tricky. Edit: and you didn't mention images storage

            Making a solution that works across editors

            Would be nice to write the server side with this in mind, but I don't think it should be a priority.

            Show
            Dan Poltawski added a comment - - edited Great question Fred, I agree with Koen's comment. Some reflections based on your technical prompts: The privacy of a draft content stored offline It strikes me that for the sake of privacy, the best thing we can do is only use local storage in exceptional circumstances - when we can't save a draft to the server due to connection/session problems. The load on the server when sending the content back and forth Handling diffs of content to save bandwidth My advice would be to avoid premature optimisation with both of these. The extra load on the database is worth considering. But In short, do a small number of limited database queries and let the database server do what we pay it to do. It should be possible for most setups to deal with this comfortably and from what we know, they are doing with quiz-autosave which has the very worst of concurrency. Where we get into major problems with Moodle performance is when we start doing 100s of queries per page, or complex expensive queries. This isn't one of these cases. I think diffing of text content is a misnomer when you consider the relative size of one image compared to gzip'd text content, I wouldn't even consider it. Clearly identifying the page/form/field on which the user is inputting content (the URL + form ID + element ID is not always enough) Yup. Agreed, this is very tricky. Edit: and you didn't mention images storage Making a solution that works across editors Would be nice to write the server side with this in mind, but I don't think it should be a priority.
            Damyon Wiese made changes -
            Assignee Damyon Wiese [ damyon ]
            Hide
            Damyon Wiese added a comment -

            About to start working on the Atto version of this.

            My early thoughts are:

            • No visual indication is required on save - that's just distracting noise
            • Local storage has privacy implications - it may be useful in cases that the server goes away and comes back (wifi) - but I think that should be a "stage 2" improvement and there are lots of ajax calls that could fail if the wifi goes away (ie just fixing this one thing does not mean that Moodle works offline)
            • Images are already on the server (in the drafts file) so it should be a case of copying all the images from the previous draft folder to the new one when the draft is "resumed"
            • Lets just do this for Atto first - the server side should consist of just a small ajax script to do the save / restore

            Things not clear to me at this stage:

            • how to identify the form instance (userid, formid, context, component, itemid)
            • how often to trigger a save (maybe every 5 seconds - only if there has been a change to the content)
            Show
            Damyon Wiese added a comment - About to start working on the Atto version of this. My early thoughts are: No visual indication is required on save - that's just distracting noise Local storage has privacy implications - it may be useful in cases that the server goes away and comes back (wifi) - but I think that should be a "stage 2" improvement and there are lots of ajax calls that could fail if the wifi goes away (ie just fixing this one thing does not mean that Moodle works offline) Images are already on the server (in the drafts file) so it should be a case of copying all the images from the previous draft folder to the new one when the draft is "resumed" Lets just do this for Atto first - the server side should consist of just a small ajax script to do the save / restore Things not clear to me at this stage: how to identify the form instance (userid, formid, context, component, itemid) how often to trigger a save (maybe every 5 seconds - only if there has been a change to the content)
            Hide
            Damyon Wiese added a comment -

            Also - not clear is if this should be added to Atto directly or written as an Atto plugin (Adding it to the toolbar seems clunky if it has no buttons).

            Show
            Damyon Wiese added a comment - Also - not clear is if this should be added to Atto directly or written as an Atto plugin (Adding it to the toolbar seems clunky if it has no buttons).
            Hide
            Tim Hunt added a comment -

            Damyon, many or your early thoughts are things I worked out (finished thoughts) in the quiz autosave code. I suggest you read that code, and don't re-invent the wheel.

            Also, no point both quiz autosave and atto autosave saving the same text. You will need to work out how to prevent that.

            Show
            Tim Hunt added a comment - Damyon, many or your early thoughts are things I worked out (finished thoughts) in the quiz autosave code. I suggest you read that code, and don't re-invent the wheel. Also, no point both quiz autosave and atto autosave saving the same text. You will need to work out how to prevent that.
            Hide
            Damyon Wiese added a comment -

            Thanks Tim,

            Yes - I will check the quiz auto save code.

            I guess there will need to be an option per-form to disable this autosave so that it can be disabled for the quiz.

            Show
            Damyon Wiese added a comment - Thanks Tim, Yes - I will check the quiz auto save code. I guess there will need to be an option per-form to disable this autosave so that it can be disabled for the quiz.
            Damyon Wiese made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            Damyon Wiese made changes -
            Hide
            Damyon Wiese added a comment -

            Here is a start.

            It saves the text every 5 seconds (only if it has been modified) and relies on the draft files still existing in the previous draft area (they are copied to the new area). The "Key" is the (elementid, pageid, userid, contextid). It is possible to disable the autosave by passing 'autosave' => false as options to the editor (and this is done in the essay renderer and the question behaviour renderer).

            Running behat against this now to see if the new confirm is accidentally triggered anywhere.

            Show
            Damyon Wiese added a comment - Here is a start. It saves the text every 5 seconds (only if it has been modified) and relies on the draft files still existing in the previous draft area (they are copied to the new area). The "Key" is the (elementid, pageid, userid, contextid). It is possible to disable the autosave by passing 'autosave' => false as options to the editor (and this is done in the essay renderer and the question behaviour renderer). Running behat against this now to see if the new confirm is accidentally triggered anywhere.
            Damyon Wiese made changes -
            Testing Instructions Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.
            Hide
            Damyon Wiese added a comment -

            Behat failed:

            (: failed steps (:

            01. The "(//html/descendant-or-self::*[@class and contains(concat(' ', normalize-space(@class), ' '), ' mod-assign-history-link ')])[1]" xpath node is not visible and it should be visible
            In step `And I click on ".mod-assign-history-link" "css_element"'. # behat_general::i_click_on()
            From scenario `Edit feedback for a students previous attempt.'. # /home/damyonw/Documents/Moodle/instances/im/moodle/mod/assign/tests/behat/edit_previous_feedback.feature:8
            Of feature `In an assignment, teachers can edit feedback for a students previous submission attempt'

            Show
            Damyon Wiese added a comment - Behat failed: (: failed steps (: 01. The "(//html/descendant-or-self::* [@class and contains(concat(' ', normalize-space(@class), ' '), ' mod-assign-history-link ')] ) [1] " xpath node is not visible and it should be visible In step `And I click on ".mod-assign-history-link" "css_element"'. # behat_general::i_click_on() From scenario `Edit feedback for a students previous attempt.'. # /home/damyonw/Documents/Moodle/instances/im/moodle/mod/assign/tests/behat/edit_previous_feedback.feature:8 Of feature `In an assignment, teachers can edit feedback for a students previous submission attempt'
            Hide
            Dan Poltawski added a comment -

            No visual indication is required on save - that's just distracting noise

            I am not sure you should dismiss this so readily, I think it has a use, even if its just a psychological reassurance that users can feel safe writing a long post. Given that they previously might've learnt not to trust that text box I think there is even more rationale to give some indication that this is now happening.
            I tried to Google Docs/ Gmail, Wordpress and Apple Pages web version and they all gave some indication when a save had happened, so there is also some existing precent for this. Note I also tried github when creating an issue, which didn't give an indication - I didn't expect my content to be there, but it was there when I went back to create the issue.

            So my arguments in favour of this are:

            1. We are trying to actively change user trust of the fields/our software
            2. Jakob's Law of the Internet User Experience
            3. Its useful for promoting Moodle/atto if the feature is more prominent. This way people get a warm glow about it when they don't need it rather than the rare time they do need it
            Show
            Dan Poltawski added a comment - No visual indication is required on save - that's just distracting noise I am not sure you should dismiss this so readily, I think it has a use, even if its just a psychological reassurance that users can feel safe writing a long post. Given that they previously might've learnt not to trust that text box I think there is even more rationale to give some indication that this is now happening. I tried to Google Docs/ Gmail, Wordpress and Apple Pages web version and they all gave some indication when a save had happened, so there is also some existing precent for this. Note I also tried github when creating an issue, which didn't give an indication - I didn't expect my content to be there, but it was there when I went back to create the issue. So my arguments in favour of this are: We are trying to actively change user trust of the fields/our software Jakob's Law of the Internet User Experience Its useful for promoting Moodle/atto if the feature is more prominent. This way people get a warm glow about it when they don't need it rather than the rare time they do need it
            Hide
            Tim Hunt added a comment -

            Thanks for working on this Damyon.

            Some randome thoughts on the patch as it currently stands:

            1. I think the 'concurrent access from the same user is not supported' restriction needs to be addressed. I think drafting two replies for the same forum in two different browser tabs is a fairly common use case. (At least, I probably do it a couple of times a week on Moodle.org.)
            2. Since you have to get involved with draft files id, can you use that to distinguish instances? ... No, I guess not
            3. I don't think a huge, mulitiply nested if, is the nicest way to structure autosave-ajax.php. I suppose I should not just criticise without suggesting something better. I think that autosave-ajax.php should just have the minimal requiest parsing code. It should call methods on a class to do all the real work. That would make it easy to unit-test the important PHP code.
            4. manual_comment_fields. Manual grading does not happen during the quiz attempt, which is the only place where quiz auto-save happens, so you should probably revert the change in question/behaviour/rendererbase.php.
            5. I would not hard-code '5 seconds' in the table comment.
            6. 'Disable the autosave because quiz has it's own auto save function.' - note that you have two very similar comments, but with unnecessary differences. Anyway, you could clarify this to make the first one say 'text-editor autosave'.
            Show
            Tim Hunt added a comment - Thanks for working on this Damyon. Some randome thoughts on the patch as it currently stands: I think the 'concurrent access from the same user is not supported' restriction needs to be addressed. I think drafting two replies for the same forum in two different browser tabs is a fairly common use case. (At least, I probably do it a couple of times a week on Moodle.org.) Since you have to get involved with draft files id, can you use that to distinguish instances? ... No, I guess not I don't think a huge, mulitiply nested if, is the nicest way to structure autosave-ajax.php. I suppose I should not just criticise without suggesting something better. I think that autosave-ajax.php should just have the minimal requiest parsing code. It should call methods on a class to do all the real work. That would make it easy to unit-test the important PHP code. manual_comment_fields. Manual grading does not happen during the quiz attempt, which is the only place where quiz auto-save happens, so you should probably revert the change in question/behaviour/rendererbase.php. I would not hard-code '5 seconds' in the table comment. 'Disable the autosave because quiz has it's own auto save function.' - note that you have two very similar comments, but with unnecessary differences. Anyway, you could clarify this to make the first one say 'text-editor autosave'.
            Hide
            Andrew Davis added a comment -

            No visual indication is required on save - that's just distracting noise

            I think we need to either indicate success (thus reassuring the user that they can trust Moodle not to lose their data) or indicate failure (thus warning them that something is wrong and that they may be about to lose data) or both.

            If the ajax autosave is failing that probably indicates that, if they hit save, they will lose data so it might be worth displaying something that conveys "connection lost".

            Show
            Andrew Davis added a comment - No visual indication is required on save - that's just distracting noise I think we need to either indicate success (thus reassuring the user that they can trust Moodle not to lose their data) or indicate failure (thus warning them that something is wrong and that they may be about to lose data) or both. If the ajax autosave is failing that probably indicates that, if they hit save, they will lose data so it might be worth displaying something that conveys "connection lost".
            Hide
            Damyon Wiese added a comment -

            -1 for indicating success (that's just like saying "everything is awesome" over and over again - annoying).

            However I do agree we should do something about errors - this is an indicator that they are about to lose some data.

            Show
            Damyon Wiese added a comment - -1 for indicating success (that's just like saying "everything is awesome" over and over again - annoying). However I do agree we should do something about errors - this is an indicator that they are about to lose some data.
            Hide
            Damyon Wiese added a comment -

            We discussed how to present a notification in this case and came up with this mockup. The goals of this mockup are:

            • Needs to be obvious which editor the notice relates to.
            • Needs to be visible when either the top or the bottom of the editor is not in view.
            • Needs to not cause the page content to jiggle.
            • Needs to be accessible.
            • Needs to not steal focus or require any interaction.
            Show
            Damyon Wiese added a comment - We discussed how to present a notification in this case and came up with this mockup. The goals of this mockup are: Needs to be obvious which editor the notice relates to. Needs to be visible when either the top or the bottom of the editor is not in view. Needs to not cause the page content to jiggle. Needs to be accessible. Needs to not steal focus or require any interaction.
            Damyon Wiese made changes -
            Attachment notices.jpg [ 39550 ]
            Hide
            Martin Dougiamas added a comment -

            For a server-based version, I could see a very little ajaxy spinning icon somewhere while it's saving, with a tooltip on it that says "Auto-saving draft..."? And not more than once a minute, I think. Losing even 60 seconds of typing is really not a big deal and we don't want to hit the server unless we need to.

            For a client version of course it can be more often and I agree that any kind of moving feedback every 5 seconds is likely to be quite annoying. Possibly a little static green light that remains green while everything is OK (with a tooltip "Autosave of drafts is active"). It would turn orange when there was a problem (tooltip something like "Drafts are not being saved automatically").

            Show
            Martin Dougiamas added a comment - For a server-based version, I could see a very little ajaxy spinning icon somewhere while it's saving, with a tooltip on it that says "Auto-saving draft..."? And not more than once a minute, I think. Losing even 60 seconds of typing is really not a big deal and we don't want to hit the server unless we need to. For a client version of course it can be more often and I agree that any kind of moving feedback every 5 seconds is likely to be quite annoying. Possibly a little static green light that remains green while everything is OK (with a tooltip "Autosave of drafts is active"). It would turn orange when there was a problem (tooltip something like "Drafts are not being saved automatically").
            Hide
            Damyon Wiese added a comment -

            Andrew pointed out it should be possible to avoid the initial ajax request.

            Show
            Damyon Wiese added a comment - Andrew pointed out it should be possible to avoid the initial ajax request.
            Hide
            David Mudrak added a comment -

            +1 for Martin's suggestions. Just a LED like status green icon for client side, and common AJAX wheel and text while saving the server side. IIRC, Google docs do actually display "Saving..." or so in their top middle notification area. I did not personally find it distracting.

            Show
            David Mudrak added a comment - +1 for Martin's suggestions. Just a LED like status green icon for client side, and common AJAX wheel and text while saving the server side. IIRC, Google docs do actually display "Saving..." or so in their top middle notification area. I did not personally find it distracting.
            Hide
            Tim Hunt added a comment -

            I love the way you are all going through the same design process we went through with quiz auto-save. Please confirm that you have acutally looked at that?

            Note that Google uses a very light grey to show 'Saving ...' which I think is good. You really don't need it in-your-face that the save worked. You just been to be able to find the confirmation if you need re-assurance.

            Also, should we not be thinking in terms of elements. What I would copy is the dependency information on plugins overview page.

            That could go just along the bottom of the editor. (Though, this is were we wish Atto had a status bar.)

            Although, with quiz auto-save we reached the same conclusion as Andrew: 'If the ajax autosave is failing that probably indicates that, if they hit save, they will lose data so it might be worth displaying something that conveys "connection lost".' Hence the way that is displayed is quite promenant. More that just a little line of red text.

            I would also diagree with some of Damyon's requirements https://tracker.moodle.org/browse/MDL-18014?focusedCommentId=298670&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-298670. If you remeber that the editor content will only changed when the editor is focussed, some of them are not acutally required.

            Show
            Tim Hunt added a comment - I love the way you are all going through the same design process we went through with quiz auto-save. Please confirm that you have acutally looked at that? Note that Google uses a very light grey to show 'Saving ...' which I think is good. You really don't need it in-your-face that the save worked. You just been to be able to find the confirmation if you need re-assurance. Also, should we not be thinking in terms of elements. What I would copy is the dependency information on plugins overview page. That could go just along the bottom of the editor. (Though, this is were we wish Atto had a status bar.) Although, with quiz auto-save we reached the same conclusion as Andrew: 'If the ajax autosave is failing that probably indicates that, if they hit save, they will lose data so it might be worth displaying something that conveys "connection lost".' Hence the way that is displayed is quite promenant. More that just a little line of red text. I would also diagree with some of Damyon's requirements https://tracker.moodle.org/browse/MDL-18014?focusedCommentId=298670&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-298670 . If you remeber that the editor content will only changed when the editor is focussed, some of them are not acutally required.
            Tim Hunt made changes -
            Attachment OK and warning.png [ 39562 ]
            Hide
            Damyon Wiese added a comment -

            If we add a notification system to Atto - it needs to meet all those requirements Tim. It would not just be used for this one thing.

            Show
            Damyon Wiese added a comment - If we add a notification system to Atto - it needs to meet all those requirements Tim. It would not just be used for this one thing.
            Hide
            Martin Dougiamas added a comment -

            Personally I've not seen quiz autosave in action ... I just made a small quiz in 2.7 using TinyMCE and I can't see anything on the attempt screens ... and I looked at MDL-14963 and I can't see any design documents at all. So, if we're going to refer to it, then it would probably be helpful for someone to post a screenshot or video showing what it's supposed to look like.

            Show
            Martin Dougiamas added a comment - Personally I've not seen quiz autosave in action ... I just made a small quiz in 2.7 using TinyMCE and I can't see anything on the attempt screens ... and I looked at MDL-14963 and I can't see any design documents at all. So, if we're going to refer to it, then it would probably be helpful for someone to post a screenshot or video showing what it's supposed to look like.
            Hide
            Tim Hunt added a comment -

            MDL-14963 has two subtasks, of which the one done so far is MDL-38538 (server-side autosave). However, that has no user-visible UI. The only user-visible bit was added in MDL-42504, which links to

            http://docs.moodle.org/27/en/Using_Quiz - there is a screen-grab there.

            We shoudl consider moving that disconnect warnig out of the quiz, and making it a core thing that can be used wherever it is needed. Presumably it should be an element.

            We realised when thinking about quiz autosave, that really if you don't have a working connection to the Moodle server, you don't want to do anything else until it is fixed, becuase you can only lose data that way. Hence a very promenant warning is right.

            Show
            Tim Hunt added a comment - MDL-14963 has two subtasks, of which the one done so far is MDL-38538 (server-side autosave). However, that has no user-visible UI. The only user-visible bit was added in MDL-42504 , which links to http://docs.moodle.org/27/en/Using_Quiz - there is a screen-grab there. We shoudl consider moving that disconnect warnig out of the quiz, and making it a core thing that can be used wherever it is needed. Presumably it should be an element. We realised when thinking about quiz autosave, that really if you don't have a working connection to the Moodle server, you don't want to do anything else until it is fixed, becuase you can only lose data that way. Hence a very promenant warning is right.
            Hide
            Martin Dougiamas added a comment -

            OK, thanks Tim.

            So quiz has no feedback at all when auto-save is working, and then a very prominent failure notice when it isn't, because we assume there is a good possibility that this is because the network is down and that an ordinary manual save would also fail. Seems reasonable to me.

            Who feels strongly about having a success indicator as well?

            Show
            Martin Dougiamas added a comment - OK, thanks Tim. So quiz has no feedback at all when auto-save is working, and then a very prominent failure notice when it isn't, because we assume there is a good possibility that this is because the network is down and that an ordinary manual save would also fail. Seems reasonable to me. Who feels strongly about having a success indicator as well?
            Hide
            Damyon Wiese added a comment -

            I feel strongly against having a success indicator.

            Show
            Damyon Wiese added a comment - I feel strongly against having a success indicator.
            Hide
            Andrew Davis added a comment -

            I wouldn't say that I feel strongly about a success indicator but there is a valid reason for having it.

            If we do not have a success indicator how will users become aware that this functionality even exists? I mean, presumably if their browser/computer crashes and they return to the page they will be pleasantly surprised to see their lost content resurrected. That is all well and good but that will only happen to a very small percentage of users. Almost from a marketing stand point I suppose, there is some value in making the user aware of the safety net we are providing even if they never need it.

            Show
            Andrew Davis added a comment - I wouldn't say that I feel strongly about a success indicator but there is a valid reason for having it. If we do not have a success indicator how will users become aware that this functionality even exists? I mean, presumably if their browser/computer crashes and they return to the page they will be pleasantly surprised to see their lost content resurrected. That is all well and good but that will only happen to a very small percentage of users. Almost from a marketing stand point I suppose, there is some value in making the user aware of the safety net we are providing even if they never need it.
            Hide
            Damyon Wiese added a comment -

            I think that trying to market a feature is a very bad reason to pollute the UI.

            Why exactly do users need to know this exist if it is working normally? How should that affect their decision making?

            Show
            Damyon Wiese added a comment - I think that trying to market a feature is a very bad reason to pollute the UI. Why exactly do users need to know this exist if it is working normally? How should that affect their decision making?
            Hide
            Dan Poltawski added a comment -

            Why exactly do users need to know this exist if it is working normally?

            Like I said above, to reassure them and change their behaviour to trust that it will be saved for them.

            Show
            Dan Poltawski added a comment - Why exactly do users need to know this exist if it is working normally? Like I said above, to reassure them and change their behaviour to trust that it will be saved for them.
            Hide
            Martin Dougiamas added a comment -

            The main reason I can think of is that it might save some wailing and gnashing in the case that something does go down. They'll know they just need to go back into the same forum and start a new post and they will find all the old text waiting, right? Otherwise they might just give up ...

            The notification doesn't need to be constant ... it could be a small initial notice "Drafts are being auto-saved" that fades away perhaps?

            On the other hand I don't feel strongly about this and perhaps we could add it later based on feedback.

            Show
            Martin Dougiamas added a comment - The main reason I can think of is that it might save some wailing and gnashing in the case that something does go down. They'll know they just need to go back into the same forum and start a new post and they will find all the old text waiting, right? Otherwise they might just give up ... The notification doesn't need to be constant ... it could be a small initial notice "Drafts are being auto-saved" that fades away perhaps? On the other hand I don't feel strongly about this and perhaps we could add it later based on feedback.
            Hide
            David Mudrak added a comment -

            On the other hand I don't feel strongly about this and perhaps we could add it later based on feedback.

            Or perhaps we could remove it later based on feedback. I think it's only fair to notify users that the draft is being auto-saved. If nothing else then only to avoid surprises when they eventually change mind and decide not to publish that post they started to type; and then they come to the same forum again. The need to know why their previous post is still there.

            Show
            David Mudrak added a comment - On the other hand I don't feel strongly about this and perhaps we could add it later based on feedback. Or perhaps we could remove it later based on feedback. I think it's only fair to notify users that the draft is being auto-saved. If nothing else then only to avoid surprises when they eventually change mind and decide not to publish that post they started to type; and then they come to the same forum again. The need to know why their previous post is still there.
            Hide
            Damyon Wiese added a comment -

            At the point when they return to the forum they will get a clear messaging saying that an unsaved draft has been found and do they want to recover it.

            Show
            Damyon Wiese added a comment - At the point when they return to the forum they will get a clear messaging saying that an unsaved draft has been found and do they want to recover it.
            Hide
            Tim Hunt added a comment -

            Just to note that Twitter does completely silent saving, and restore when you go back in (without even asking, but then the twitter UI is simple enough that they probably don't need to ask).

            Show
            Tim Hunt added a comment - Just to note that Twitter does completely silent saving, and restore when you go back in (without even asking, but then the twitter UI is simple enough that they probably don't need to ask).
            Hide
            Alan Arnold added a comment - - edited

            While I appreciate your concerns about complicating the UI Damyon (and luv your work here's a use case where a "saved" indicator would have been really appreciated. Yesterday I was asked to add some words to a collaborative MS Word document on live.onedrive.com. After spending half hour doing this, I had to close down the laptop and move to yet-another-meeting. Moment of anxiety - as far as I could tell there was no indication that my work had been saved, so I copied and pasted the whole lot into an email to myself. I really would have preferred to have had the confidence that my work had been saved before closing down that browser.

            Who says I'm paranoid?

            Show
            Alan Arnold added a comment - - edited While I appreciate your concerns about complicating the UI Damyon (and luv your work here's a use case where a "saved" indicator would have been really appreciated. Yesterday I was asked to add some words to a collaborative MS Word document on live.onedrive.com. After spending half hour doing this, I had to close down the laptop and move to yet-another-meeting. Moment of anxiety - as far as I could tell there was no indication that my work had been saved, so I copied and pasted the whole lot into an email to myself. I really would have preferred to have had the confidence that my work had been saved before closing down that browser. Who says I'm paranoid?
            Hide
            Andrew Davis added a comment -

            I think that trying to market a feature is a very bad reason to pollute the UI.

            Its not about marketing a feature, its about marketing Moodle.

            This is the reason that cars have airbags and why car companies make sure to tell you about them. It makes zero difference to Toyota if you die in an accident but you knowing that systems to protect you exist changes how you feel about the product. It doesn't matter that 99% of people will never need them. 100% of people gain a psychological benefit from simply knowing that they are being looked after. You do however have to know that these safety systems exist for that to happen.

            Show
            Andrew Davis added a comment - I think that trying to market a feature is a very bad reason to pollute the UI. Its not about marketing a feature, its about marketing Moodle. This is the reason that cars have airbags and why car companies make sure to tell you about them. It makes zero difference to Toyota if you die in an accident but you knowing that systems to protect you exist changes how you feel about the product. It doesn't matter that 99% of people will never need them. 100% of people gain a psychological benefit from simply knowing that they are being looked after. You do however have to know that these safety systems exist for that to happen.
            Hide
            Martin Dougiamas added a comment -

            What places are possible, Damyon, if we're thinking about possible unobtrusive locations for some sort of positive feedback? Can Atto plugins spill out of the actual editor area?

            Worth noting the gmail system in the footer of the editor:

            "Saved" – shown after a save is successful and while the editor contents = saved contents
            "Saving" - during the process of updating save buffer with new editor contents (in a typing pause, or every 10 seconds)
            " " - during the time when your editor contents are ahead of the buffer (ie while typing)

            This does work pretty well ... but I can't see room for that within the ATTO UI itself, and hanging it off an edge would just look fugly.

            Guess I'm returning to a small coloured dot or corner-triangle somewhere that changes colour ...

            Otherwise one small quick temporary notification that pops up when you start editing: "This text is being auto-saved..."

            Show
            Martin Dougiamas added a comment - What places are possible, Damyon, if we're thinking about possible unobtrusive locations for some sort of positive feedback? Can Atto plugins spill out of the actual editor area? Worth noting the gmail system in the footer of the editor: "Saved" – shown after a save is successful and while the editor contents = saved contents "Saving" - during the process of updating save buffer with new editor contents (in a typing pause, or every 10 seconds) " " - during the time when your editor contents are ahead of the buffer (ie while typing) This does work pretty well ... but I can't see room for that within the ATTO UI itself, and hanging it off an edge would just look fugly. Guess I'm returning to a small coloured dot or corner-triangle somewhere that changes colour ... Otherwise one small quick temporary notification that pops up when you start editing: "This text is being auto-saved..."
            Hide
            Dan Poltawski added a comment -

            I'm totally with Andrew Davis comment.

            If you want to make it mega-subtle, you could copy the apple approach and put some text in the titlebar.

            Show
            Dan Poltawski added a comment - I'm totally with Andrew Davis comment. If you want to make it mega-subtle, you could copy the apple approach and put some text in the titlebar.
            Hide
            Damyon Wiese added a comment -

            When I get back to looking at it I'll do some screenshots of alternatives.

            Show
            Damyon Wiese added a comment - When I get back to looking at it I'll do some screenshots of alternatives.
            Hide
            Damyon Wiese added a comment -

            Here is a screen shot of my current solution.

            Show
            Damyon Wiese added a comment - Here is a screen shot of my current solution.
            Damyon Wiese made changes -
            Attachment shot.png [ 39646 ]
            Hide
            Joseph Thibault added a comment -
            Show
            Joseph Thibault added a comment - Damyon Wiese lol.
            Damyon Wiese made changes -
            Testing Instructions Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window. Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

            Open a page with an Atto editor.
            Type some text - wait more than 60 seconds.
            Leave the page without submitting the form.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click recover, verify you got the unsaved edits.
            Make some more edits, wait more than 60 seconds.
            Leave the page without saving.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click cancel, verify you did not get the unsaved edits.
            With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
            Start apache again, verify the warning goes away after 60 seconds.
            With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.
            Affects Version/s FRONTEND [ 12581 ]
            Hide
            Damyon Wiese added a comment -

            So the current solution in this issue is this:

            • do not show any indication when auto-saving
            • if there is an error when auto-saving show a big obvious warning
            • if returning to a form with autosaved text show a big modal "confirm recovery" dialogue

            I think this version is ready for a peer review - I'm running behat on it now to confirm it doesn't trigger any bugs.

            Show
            Damyon Wiese added a comment - So the current solution in this issue is this: do not show any indication when auto-saving if there is an error when auto-saving show a big obvious warning if returning to a form with autosaved text show a big modal "confirm recovery" dialogue I think this version is ready for a peer review - I'm running behat on it now to confirm it doesn't trigger any bugs.
            Damyon Wiese made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            CiBoT made changes -
            Labels triaged ci triaged
            Hide
            CiBoT added a comment -

            Fails against automated checks.

            Checked MDL-18014 using repository: git://github.com/damyon/moodle.git

            More information about this report

            Show
            CiBoT added a comment - Fails against automated checks. Checked MDL-18014 using repository: git://github.com/damyon/moodle.git master (branch: MDL-18014-master | CI Job ) Coding style problems found More information about this report
            Dan Poltawski made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Peer reviewer Dan Poltawski [ poltawski ]
            Hide
            Dan Poltawski added a comment - - edited

            Hi Damyon.

            I'll start by repeating myself, having played with what you've done I still wholeheartedly believe we should have something which indicates autosave is happening. Not in your face, subtle but allows some reassurance/builds trust. This is more important to me than the actual feature.. But ignoring that..

            1. This dialogue make me have to think:

            A previously unsaved version of the text for field "{$a->label}" was found. Do you want to recover it?
            Recover / Cancel

            a) It's not clear if 'cancel' is destructive.
            b) The label for the text field was a bit meaningless in my testing (it might be 'message' - but thats just the same as 'text' really. Tricky, so I have to wonder..
            c) Is it really worth presenting a dialogue, should it not just restore it? Could we provide an non-modal dialogue for dealing with these drafts in a button?
            d) The titlebar was way too big (on safari)

            2. I got the 'autosavefailed' message once in my limited (2/3min) testing on my local machine. Hopefully it was just a blip - but I wouldn't expect blips on my local moodle host.

            3. I think that AUTOSAVE_FREQUENCY could be configurable - it would allow admins to reduce the impact of it but keep the feature. Or make save more frequently (as I think the impact will be negligible on moodle servers)

            4. messageTypeIcon = '<img width="16" height="16" src" - any reason to hardcode these?

            5. I think this line is confusing:

            var draftid = -1, optiontype, options = this.get('filepickeroptions');
            

            6. Have you done much testing into how effective the means of identifying the editor is?

            Otherwise it looks good

            Show
            Dan Poltawski added a comment - - edited Hi Damyon. I'll start by repeating myself, having played with what you've done I still wholeheartedly believe we should have something which indicates autosave is happening. Not in your face, subtle but allows some reassurance/builds trust. This is more important to me than the actual feature.. But ignoring that.. 1. This dialogue make me have to think: A previously unsaved version of the text for field "{$a->label}" was found. Do you want to recover it? Recover / Cancel a) It's not clear if 'cancel' is destructive. b) The label for the text field was a bit meaningless in my testing (it might be 'message' - but thats just the same as 'text' really. Tricky, so I have to wonder.. c) Is it really worth presenting a dialogue, should it not just restore it? Could we provide an non-modal dialogue for dealing with these drafts in a button? d) The titlebar was way too big (on safari) 2. I got the 'autosavefailed' message once in my limited (2/3min) testing on my local machine. Hopefully it was just a blip - but I wouldn't expect blips on my local moodle host. 3. I think that AUTOSAVE_FREQUENCY could be configurable - it would allow admins to reduce the impact of it but keep the feature. Or make save more frequently (as I think the impact will be negligible on moodle servers) 4. messageTypeIcon = '<img width="16" height="16" src" - any reason to hardcode these? 5. I think this line is confusing: var draftid = -1, optiontype, options = this.get('filepickeroptions'); 6. Have you done much testing into how effective the means of identifying the editor is? Otherwise it looks good
            Dan Poltawski made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            CiBoT made changes -
            Labels ci triaged triaged
            Hide
            Damyon Wiese added a comment -

            Thanks Dan - I have pushed an update which adds a notification when the text is saved. The frequency is now configurable and the text is always recovered if found (no dialog). If the text is recovered, a notification is shown instead.

            This covers 1-5.

            I think 6 is not right yet. E.g. grading students in mod assign does not distinguish between the students (so you could have text recovered for the wrong student). Same in forum - it doesn't distinguish the post/discussion.

            Show
            Damyon Wiese added a comment - Thanks Dan - I have pushed an update which adds a notification when the text is saved. The frequency is now configurable and the text is always recovered if found (no dialog). If the text is recovered, a notification is shown instead. This covers 1-5. I think 6 is not right yet. E.g. grading students in mod assign does not distinguish between the students (so you could have text recovered for the wrong student). Same in forum - it doesn't distinguish the post/discussion.
            Hide
            Dan Poltawski added a comment -

            I have pushed an update which adds a notification when the text is saved.

            I would advocate something more subtle than that (Also, I think it would be nice to have a bit of Y.Anim fade in and out so its less jarring.)

            I much prefer the restoration behaviour now and I like the notification for that too - it looks slick.

            Show
            Dan Poltawski added a comment - I have pushed an update which adds a notification when the text is saved. I would advocate something more subtle than that (Also, I think it would be nice to have a bit of Y.Anim fade in and out so its less jarring.) I much prefer the restoration behaviour now and I like the notification for that too - it looks slick.
            Hide
            Andrew Nicols added a comment -

            Sorry, but I'm also going to put in my veto on this at the moment. There are some issues with the JS coding that I've noticed:

            1. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R48 - Integer is not a type - use Number (multiple occurrences)
            2. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R140 - I'd prefer to see this as an attribute with a default value
            3. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R159 - why sync? This will slow down page submissions considerably for slow servers. This should be async with the session discarded immediately after validation server-side
            4. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R181 (and in showMessage as a whole), I think we should use constants in the class rather than strings for info and warning
            5. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R217 - missing the log source in this. There are others around which are also missing the log level
            6. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R238 - perhaps move context above the event handler section to make it more visible?
            7. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R221 - how does failure differ from the first section of success? Better to use a complete handler and one additional test perhaps?
            8. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R20 - why notify-base? Surely the submodule is notify?
            9. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R37 - shoudl by Overlay, not Y.Overlay for correct documentation generation
            10. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R69 - please don't use hyphens between the variable and description. No punctuation necessary
            11. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R75 - random whitespace
            12. https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R37 - it's not actually an Overlay at all - it's a Node as defined on line 77

            Cheers,

            Andrew

            Show
            Andrew Nicols added a comment - Sorry, but I'm also going to put in my veto on this at the moment. There are some issues with the JS coding that I've noticed: https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R48 - Integer is not a type - use Number (multiple occurrences) https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R140 - I'd prefer to see this as an attribute with a default value https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R159 - why sync? This will slow down page submissions considerably for slow servers. This should be async with the session discarded immediately after validation server-side https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R181 (and in showMessage as a whole), I think we should use constants in the class rather than strings for info and warning https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R217 - missing the log source in this. There are others around which are also missing the log level https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R238 - perhaps move context above the event handler section to make it more visible? https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-32edad94bae57ff6f456d9720ce62100R221 - how does failure differ from the first section of success? Better to use a complete handler and one additional test perhaps? https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R20 - why notify-base? Surely the submodule is notify? https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R37 - shoudl by Overlay, not Y.Overlay for correct documentation generation https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R69 - please don't use hyphens between the variable and description. No punctuation necessary https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R75 - random whitespace https://github.com/damyon/moodle/compare/37c2490aa365c56b9b396fefe7c4b1f919ebf201...MDL-18014-master#diff-c3cb144006c8f854e3a386af58cdc670R37 - it's not actually an Overlay at all - it's a Node as defined on line 77 Cheers, Andrew
            Hide
            Damyon Wiese added a comment -

            Andrew -
            1. fixed
            2. that sounds backwards to me - it's a function that does something not an attribute with side effects
            3. because this is part of the form submission and the ajax request would be cancelled before it gets to the server
            4. fixed - (this didn't work the first time I tried, but it's working now so whatever)
            5. fixed
            6. fixed
            7. the differences are a) the server could not be contacted at all and the ajax request failed completely, b) the server was contacted, but there was an error response (session is no longer valid etc). I removed the duplication of the errorHandler.
            8. changed
            9. changed
            10. changed
            11. changed
            12. changed

            Show
            Damyon Wiese added a comment - Andrew - 1. fixed 2. that sounds backwards to me - it's a function that does something not an attribute with side effects 3. because this is part of the form submission and the ajax request would be cancelled before it gets to the server 4. fixed - (this didn't work the first time I tried, but it's working now so whatever) 5. fixed 6. fixed 7. the differences are a) the server could not be contacted at all and the ajax request failed completely, b) the server was contacted, but there was an error response (session is no longer valid etc). I removed the duplication of the errorHandler. 8. changed 9. changed 10. changed 11. changed 12. changed
            Hide
            Andrew Nicols added a comment -

            Re 2: I meant the path. It would give the possibility to specify an alternative URL in some situations.

            Show
            Andrew Nicols added a comment - Re 2: I meant the path. It would give the possibility to specify an alternative URL in some situations.
            Hide
            Martin Dougiamas added a comment -

            I just had a demo from Damyon of the latest code. Only suggestion I could make was to shorten the notification string during a save to "Draft saved" so as to make it a little less distracting, but otherwise I agree it looks really slick now. +1

            Show
            Martin Dougiamas added a comment - I just had a demo from Damyon of the latest code. Only suggestion I could make was to shorten the notification string during a save to "Draft saved" so as to make it a little less distracting, but otherwise I agree it looks really slick now. +1
            Hide
            Damyon Wiese added a comment -

            Re: 2 - ah - I thought you meant the whole function. changed.

            Show
            Damyon Wiese added a comment - Re: 2 - ah - I thought you meant the whole function. changed.
            Hide
            Damyon Wiese added a comment -

            This is almost ready for integration - but not quite. There is still this outstanding:

            "I think 6 is not right yet. E.g. grading students in mod assign does not distinguish between the students (so you could have text recovered for the wrong student). Same in forum - it doesn't distinguish the post/discussion."

            Show
            Damyon Wiese added a comment - This is almost ready for integration - but not quite. There is still this outstanding: "I think 6 is not right yet. E.g. grading students in mod assign does not distinguish between the students (so you could have text recovered for the wrong student). Same in forum - it doesn't distinguish the post/discussion."
            Damyon Wiese made changes -
            Sprint FRONTEND Sprint 13 [ 26 ] BACKEND Sprint 15 [ 28 ]
            Dan Poltawski made changes -
            Labels triaged docs_required triaged ui_change
            Hide
            Damyon Wiese added a comment -

            Stick a fork in it - I'm calling this done.

            The last change was to fix the "uniqueness" problem by using a hash of the current $PAGE->url.

            Show
            Damyon Wiese added a comment - Stick a fork in it - I'm calling this done. The last change was to fix the "uniqueness" problem by using a hash of the current $PAGE->url.
            Damyon Wiese made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Damyon Wiese made changes -
            Testing Instructions Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

            Open a page with an Atto editor.
            Type some text - wait more than 60 seconds.
            Leave the page without submitting the form.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click recover, verify you got the unsaved edits.
            Make some more edits, wait more than 60 seconds.
            Leave the page without saving.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click cancel, verify you did not get the unsaved edits.
            With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
            Start apache again, verify the warning goes away after 60 seconds.
            With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.
            Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

            Open a page with an Atto editor.
            Type some text - wait more than 60 seconds.
            Leave the page without submitting the form.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click recover, verify you got the unsaved edits.
            Make some more edits, wait more than 60 seconds.
            Leave the page without saving.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click cancel, verify you did not get the unsaved edits.
            With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
            Start apache again, verify the warning goes away after 60 seconds.
            With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.

            Load a forum - make 2 posts. Start replying to the first post, then wait until the "Draft saved" message is shown. Then leave the page without saving the form. Start replying to the second post and verify the draft is not recovered for the wrong post. Go back and start replying to the first post and ensure the draft is recovered because it matches this post.
            Tim Hunt made changes -
            Testing Instructions Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

            Open a page with an Atto editor.
            Type some text - wait more than 60 seconds.
            Leave the page without submitting the form.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click recover, verify you got the unsaved edits.
            Make some more edits, wait more than 60 seconds.
            Leave the page without saving.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click cancel, verify you did not get the unsaved edits.
            With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
            Start apache again, verify the warning goes away after 60 seconds.
            With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.

            Load a forum - make 2 posts. Start replying to the first post, then wait until the "Draft saved" message is shown. Then leave the page without saving the form. Start replying to the second post and verify the draft is not recovered for the wrong post. Go back and start replying to the first post and ensure the draft is recovered because it matches this post.
            Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

            Open a page with an Atto editor.
            Type some text - wait more than 60 seconds.
            Leave the page without submitting the form.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click recover, verify you got the unsaved edits.
            Make some more edits, wait more than 60 seconds.
            Leave the page without saving.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click cancel, verify you did not get the unsaved edits.
            With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
            Start apache again, verify the warning goes away after 60 seconds.
            With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.

            Load a forum - make 2 posts. Start replying to the first post, then wait until the "Draft saved" message is shown. Then leave the page without saving the form. Start replying to the second post and verify the draft is not recovered for the wrong post. Go back and start replying to the first post and ensure the draft is recovered because it matches this post.

            Another potentially tricky case:
            # Create a quiz with an Essay question in it.
            # Attempt the quiz as two different students.
            # Go to Results -> Manual grading, and select the essay question.
            # Verify that autosave works on the two editors on that page.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            CiBoT made changes -
            Status Waiting for integration review [ 10010 ] Waiting for integration review [ 10010 ]
            Currently in integration Yes [ 10041 ]
            Hide
            CiBoT added a comment -

            Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

            Show
            CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            CiBoT made changes -
            Labels docs_required triaged ui_change ci docs_required triaged ui_change
            Marina Glancy made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator Marina Glancy [ marina ]
            Hide
            Marina Glancy added a comment - - edited

            Hi, this is definitely a great feature and all those nice popups look amazing.
            But it really need to be reversible, so when the message "was automatically restored" appears, there should be a link "Cancel".

            To prove you here is the interesting case:

            1. Create a label with an image, save it.
            2. Edit the label, add some text to it, wait 60 sec and close the window without saving
            3. Adjust the clock on the server (assuming it's your localhost) to be 5 days ahead, or wait for 5 days if you wish
            4. Run cron. This will delete draft file areas that are more than 4 days old
            5. Go to this label and start editing it - you won't see the images that existed there BEFORE your changes.

            The problem here is that the files in filearea may be not images but linked files or videos and it won't be obvious that they are lost on re-editing. Saving such form will result in loosing content. It will be sensible to make lifespan of editor drafts less than lifespan of files draft area.

            Show
            Marina Glancy added a comment - - edited Hi, this is definitely a great feature and all those nice popups look amazing. But it really need to be reversible, so when the message "was automatically restored" appears, there should be a link "Cancel". To prove you here is the interesting case: Create a label with an image, save it. Edit the label, add some text to it, wait 60 sec and close the window without saving Adjust the clock on the server (assuming it's your localhost) to be 5 days ahead, or wait for 5 days if you wish Run cron. This will delete draft file areas that are more than 4 days old Go to this label and start editing it - you won't see the images that existed there BEFORE your changes. The problem here is that the files in filearea may be not images but linked files or videos and it won't be obvious that they are lost on re-editing. Saving such form will result in loosing content. It will be sensible to make lifespan of editor drafts less than lifespan of files draft area.
            Hide
            Damyon Wiese added a comment -

            Hi Marina - I added 2 commits.

            1 to force something into the undo stack when text is restored. This allows you to use the "undo" button to go back to the original text in the form.

            2 to add a timemodified column to the drafts table, and never restore drafts older than 4 days. This is not a perfect sync with the draft file lifetime - but it's better than nothing.

            Show
            Damyon Wiese added a comment - Hi Marina - I added 2 commits. 1 to force something into the undo stack when text is restored. This allows you to use the "undo" button to go back to the original text in the form. 2 to add a timemodified column to the drafts table, and never restore drafts older than 4 days. This is not a perfect sync with the draft file lifetime - but it's better than nothing.
            Hide
            Marina Glancy added a comment -

            Thanks Damyon, you made me happy.
            Integrated in master

            Show
            Marina Glancy added a comment - Thanks Damyon, you made me happy. Integrated in master
            Marina Glancy made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Fix Version/s 2.8 [ 13150 ]
            Fix Version/s DEV backlog [ 10464 ]
            Fix Version/s FRONTEND [ 12581 ]
            Hide
            Damyon Wiese added a comment -

            Note this caused some unit test fails. There is a final commit on the branch to fix the failures. The failures were caused by the use of $PAGE->url which is a magic method that throws debugging if $PAGE->set_url has not been called.

            I considered modifying lib/pagelib.php to not throw the debugging if we are in a unit test, but that seemed like a blanket fix and I can't say this warning will never catch something in a unit test (but then I can't think of how it would). Instead I called $PAGE->set_url in the failing tests.

            Show
            Damyon Wiese added a comment - Note this caused some unit test fails. There is a final commit on the branch to fix the failures. The failures were caused by the use of $PAGE->url which is a magic method that throws debugging if $PAGE->set_url has not been called. I considered modifying lib/pagelib.php to not throw the debugging if we are in a unit test, but that seemed like a blanket fix and I can't say this warning will never catch something in a unit test (but then I can't think of how it would). Instead I called $PAGE->set_url in the failing tests.
            Hide
            Marina Glancy added a comment -

            Thanks Damyon, I think it is a reasonable fix for tests that analyze html generated on a page

            Show
            Marina Glancy added a comment - Thanks Damyon, I think it is a reasonable fix for tests that analyze html generated on a page
            Rajesh Taneja made changes -
            Tester Adrian Greeve [ abgreeve ]
            Adrian Greeve made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Damyon Wiese made changes -
            Testing Instructions Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

            Open a page with an Atto editor.
            Type some text - wait more than 60 seconds.
            Leave the page without submitting the form.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click recover, verify you got the unsaved edits.
            Make some more edits, wait more than 60 seconds.
            Leave the page without saving.
            Go back to the page.
            Verify you got a confirm asking if you want to recover the previous text.
            Click cancel, verify you did not get the unsaved edits.
            With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
            Start apache again, verify the warning goes away after 60 seconds.
            With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.

            Load a forum - make 2 posts. Start replying to the first post, then wait until the "Draft saved" message is shown. Then leave the page without saving the form. Start replying to the second post and verify the draft is not recovered for the wrong post. Go back and start replying to the first post and ensure the draft is recovered because it matches this post.

            Another potentially tricky case:
            # Create a quiz with an Essay question in it.
            # Attempt the quiz as two different students.
            # Go to Results -> Manual grading, and select the essay question.
            # Verify that autosave works on the two editors on that page.
            Run the entire behat suite. We are looking for any tests that accidentally trigger this "recovery" confirm window.

            Open a page with an Atto editor.
            Type some text - wait more than 60 seconds.
            Leave the page without submitting the form.
            Go back to the page.
            Verify a small notification shows in the editor and the unsaved text was recovered.

            With the editor open, shutdown apache and make some changes to the text. Verify you get a warning after 60 seconds - "Could not connect to the server. If you submit this page now, your changes may be lost.".
            Start apache again, verify the warning goes away after 60 seconds.
            With the editor open in a page, in another tab, logout the current session. Make some changes to the text, verify after 60 seconds you get the same warning.

            Load a forum - make 2 posts. Start replying to the first post, then wait until the "Draft saved" message is shown. Then leave the page without saving the form. Start replying to the second post and verify the draft is not recovered for the wrong post. Go back and start replying to the first post and ensure the draft is recovered because it matches this post.

            Another potentially tricky case:
            # Create a quiz with an Essay question in it.
            # Attempt the quiz as two different students.
            # Go to Results -> Manual grading, and select the essay question.
            # Verify that autosave works on the two editors on that page.
            Hide
            Damyon Wiese added a comment -

            Michael suggested this change: https://tracker.moodle.org/browse/MDL-46899

            Show
            Damyon Wiese added a comment - Michael suggested this change: https://tracker.moodle.org/browse/MDL-46899
            Hide
            Adrian Greeve added a comment -

            Everything worked as described.
            Test passed.

            Show
            Adrian Greeve added a comment - Everything worked as described. Test passed.
            Adrian Greeve made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            Marina Glancy added a comment -

            Thanks for your contribution. Your change is now upstream!

            Show
            Marina Glancy added a comment - Thanks for your contribution. Your change is now upstream!
            Marina Glancy made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 21/Aug/14
            Tim Hunt made changes -
            Link This issue caused a regression MDL-47140 [ MDL-47140 ]
            Michael de Raadt made changes -
            Labels ci docs_required triaged ui_change ci docs_required qa_test_required triaged ui_change
            Mary Cooch made changes -
            Labels ci docs_required qa_test_required triaged ui_change ci docs_required triaged ui_change
            Hide
            Mary Cooch added a comment -

            Removing qa_test_required label as I have made MDLQA-7152 ready for including in the next QA cycle. (Docs will be updated when we have 2.8 docs)

            Show
            Mary Cooch added a comment - Removing qa_test_required label as I have made MDLQA-7152 ready for including in the next QA cycle. (Docs will be updated when we have 2.8 docs)
            Hide
            Dan Poltawski added a comment -

            When I was playing with autosave today I had a thought about a an improvement to when we might make the saves - not sure if its a good idea or not, but created MDL-47822 to think about it.

            Show
            Dan Poltawski added a comment - When I was playing with autosave today I had a thought about a an improvement to when we might make the saves - not sure if its a good idea or not, but created MDL-47822 to think about it.
            Dan Poltawski made changes -
            Link This issue has a non-specific relationship to MDL-47822 [ MDL-47822 ]
            Hide
            Mary Cooch added a comment -

            Removed docs_required label as this is documented in https://docs.moodle.org/28/en/Text_editor#Autosave

            Show
            Mary Cooch added a comment - Removed docs_required label as this is documented in https://docs.moodle.org/28/en/Text_editor#Autosave
            Mary Cooch made changes -
            Labels ci docs_required triaged ui_change ci triaged ui_change
            Damyon Wiese made changes -
            Link This issue will help resolve MDL-46194 [ MDL-46194 ]
            Dan Poltawski made changes -
            Labels ci triaged ui_change ci dev_docs_required triaged ui_change
            Hide
            Dan Poltawski added a comment -

            I've added dev_docs_required to this issue as mentioned by Yuliya Bozhko - there does not seem to be any docs on how third party developers should handle how the autosave uniqueing etc works.

            Show
            Dan Poltawski added a comment - I've added dev_docs_required to this issue as mentioned by Yuliya Bozhko - there does not seem to be any docs on how third party developers should handle how the autosave uniqueing etc works.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            +10^6

            Show
            Eloy Lafuente (stronk7) added a comment - +10^6

              People

              • Votes:
                84 Vote for this issue
                Watchers:
                57 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Agile