Release lock when browsing away from locked form

Created on 22 July 2020, over 4 years ago
Updated 27 March 2023, over 1 year ago

Problem/Motivation

Content lock protects against simultaneous editing. However, content remains locked when a user navigates away from the form.

Proposed resolution

Assume the content lock can be releases when a user browses away from an entity that has been locked.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

A user is no longer warned the lock remains after leaving the form.

API changes

None.

Data model changes

None.

✨ Feature request
Status

RTBC

Version

2.0

Component

Code

Created by

πŸ‡³πŸ‡±Netherlands idebr

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡³πŸ‡±Netherlands idebr

    I attached the patch version of the current merge request for those working with composer patches.

  • The last submitted patch, 35: 3160781-35.patch, failed testing. View results β†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Status changed to Needs work over 1 year ago
  • It's better not to post separate patches when using merge requests for the actual changes. It makes it confusing, as people often submit new patches rather than putting later changes into the MR. Just link to the MR patch file, which people can download.

    https://git.drupalcode.org/project/content_lock/-/merge_requests/12.patch

  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    But don't reference that merge request URL directly in your composer.json file. Its contents can change at any time, potentially silently breaking your website after new composer installs. Just keep your patch files locally, together with the rest of your website code.

  • πŸ‡§πŸ‡¬Bulgaria vuil Bulgaria πŸ‡§πŸ‡¬ πŸ‡ͺπŸ‡Ί 🌍
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Like the idea but feels like something that should be a form setting or another submodule like timeout is? thoughts.

  • It should probably be configurable with a setting. However, for content editors who have no idea about Drupal or its functionality, who just manage their content, they might have no idea what "Break lock" means, or they might forget if they do know. I think it's a good feature to add.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    But it might not be a setting that all sites want.

  • Sure, anyone could turn off the setting.

  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    I agree @solideogloria, I have been using this patch for about 2 years now on a website where I'm an editor, and in my opinion it's a very valuable feature. It would be good to get this in the module at some point. I couldn't imagine always having to unlock the form manually.

  • First commit to issue fork.
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    I've rebased MR!12, should probably be tested again.

  • Pipeline finished with Failed
    6 months ago
    Total: 178s
    #173457
  • Pipeline finished with Success
    6 months ago
    Total: 230s
    #173456
  • There is a test failure. Is that related?

  • Also, I'm not sure if this fix is working at all. I have the latest changes applied to a site, I edit a node (locking it), and then I either click "view" to leave the editing screen or close the tab, but the content is still locked.

  • πŸ‡¨πŸ‡¦Canada joelseguin Ontario, Canada

    I can't seem to be able to apply the patch via composer to test.

  • You have to be using the dev version of the module. It doesn't apply to the latest release.

  • πŸ‡·πŸ‡ΈSerbia levmyshkin Novi Sad, Serbia

    I couldn't applied #35 patch for dev or 2.4 version. Re-rolled patch for 2.4 version.

  • Pipeline finished with Failed
    4 months ago
    Total: 158s
    #223500
  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels
  • Pipeline finished with Failed
    4 months ago
    Total: 289s
    #224000
  • Pipeline finished with Failed
    4 months ago
    Total: 234s
    #224001
  • Pipeline finished with Failed
    4 months ago
    Total: 172s
    #224023
  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    After using this for 2+ years I have to agree that the possibility to manually unlock still needs to be present. Automatic unlocking works in 95% of the cases, but it might not work if a user has JS disabled or if for example their device/browser crashes and it doesn't ping the unlock endpoint. Maybe the button shouldn't be there on the edit form to avoid confusion, but we could e.g. add an entity operation to overviews to unlock nodes that are stuck in a locked state. We should probably add two separate settings, 'Lock form using JS' and 'Unlock form using JS' instead of 1 combined setting.

  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    Also, I just discovered the onunload event is deprecated:

    Developers should avoid using this event.

    Especially on mobile, the unload event is not reliably fired. For example, the unload event is not fired at all in the following scenario:

    A mobile user visits your page.
    The user then switches to a different app.
    Later, the user closes the browser from the app manager.

    https://developer.mozilla.org/en-US/docs/Web/API/Window/unload_event#usa...

    I'll update the MR to use one of the other recommended events.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 322s
    #298901
  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    Never mind, I can't seem to make pagehide work and I saw that beforeunload is not deprecated, so using both unload and beforeonload seems like the best way to go.

  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels

    Maybe the button shouldn't be there on the edit form to avoid confusion, but we could e.g. add an entity operation to overviews to unlock nodes that are stuck in a locked state.

    I forgot about the status message with the unlock link that shows if the user has permission to unlock. That's probably sufficient.

  • πŸ‡§πŸ‡ͺBelgium dieterholvoet Brussels
  • πŸ‡ΊπŸ‡ΈUnited States robpowell Boston

    To test this feature, I have userA in my primary window and I use google chrome's incognito window and sign in as userB.

    Unlock form using JS

    While having Unlock form using JS and verbose messaging enabled, I check out node1 as UserA. I do not see a message saying the node is checked out. When I navigate to the page as UserB I do not see the warning and in fact am able to submit the form with out issue.

    Unlock/Lock form using JS

    While having Unlock/Lock form using JS and verbose messaging enabled, I check out node1 as UserA. I do see the message saying I've checked out the node. When I navigate to the page as UserB I do see the warning. However, As UserA, when I navigate away from the page and refresh the window for UserB I still see the warning and am unable to submit the form.

    Results

    I would expect the regardless of my selection of Unlock/Lock form using JS the Verbose message will always display. Based off of my testing, navigating away from the page by either hitting the browser back or clicking the node view task, does not unlock the page.

    Local setup.

    • Drupal: 10.3
    • Content lock: 3.0.0-alpha2
    • Chrome: Version 129.0.6668.90
  • πŸ‡ΊπŸ‡¦Ukraine evgen

    Can't apply Merge request !12 for 3.x

  • Pipeline finished with Failed
    1 day ago
    Total: 219s
    #344871
Production build 0.71.5 2024