In Firefox, I open a node edit page. In a second tab of the browser, I check my current locks in my user profile and see that the node is locked.
Now I click the refresh button of the browser to refresh the node edit form. Then I refresh the list of locks. The node is now unlocked, even though I'm still on the (now refreshed) node edit page.

When I press F5 to reload the page, the lock is kept. But clicking the refresh button releases the lock, which in my view is a bug.

I guess it would be fine to release the lock and then reacquire it when the page is loaded again, but for some reason that does not happen. I'm currently debugging this to better understand the sequence of events. Maybe the ajax request to release the lock is called after the page has already been refreshed?

Comments

Bodo Maass’s picture

Bodo Maass commented

I added some watchdog messages to content_lock_node and content_lock_release. It does indeed appear to be the case that on a page refresh, the page gets locked by the new page request, and then released by the ajax request from leaving the old page, leaving the page unlocked.

I guess the best solution would be to detect in the javascript that the lock should not be released in this case. Still looking at how to implement that.

More notes: Using Fiddler, I can see that in IE first the ajax call to release the lock is called, and then the new page request. So all is fine there.
In Firefox, however, first the request for the new page is called, and then the release lock request from the old page. That sucks. Maybe the server should have a time period in which the new lock cannot be released by the same client?

Bodo Maass’s picture

Bodo Maass commented

I experimented with rejecting release_lock requests coming right after a lock request, but it is too fragile to be useful. I guess it must be enough to educate users about not refreshing their page.

ohnobinki’s picture

ohnobinki commented

There is code which tries to detect keypresses which would indicate page refreshes and not release the lock in such a case. This works for me when I use C-r to refresh a page. However, I am still able to reproduce this by just clicking the GUI ``Reload'' button in my browser.

I think the best solution would be somehow detecting a reload, but I have no idea where to find a portable way to do so. Maybe following a link in the browser would change document.location but reloading wouldn't...?

Bodo Maass’s picture

Bodo Maass commented

The code is detecting the reload fine. The problem is that Firefox first sends the request for the new page to the server (which keeps the lock), and then fires the handler on the old page, which releases the lock.

eugenmayer’s picture

eugenmayer commented

I see. Well its pretty hard to get this stuff robus. I was really debugging this for ages because of the timing. Not sure how we can fix this.

Thanks for all your investigations and debugging!

ohnobinki’s picture

ohnobinki commented
Status: Active » Needs review

Fixed with https://github.com/EugenMayer/content_lock/commit/a4cfac9077aceffcf7f48c... . Requires running update.php.

Now each time a node's Edit page is loaded, the AJAX unlocking code is given a key (ajax_key) which it must use to unlock that page. Also, a new ajax_key is generated each time a node's edit page is loaded. Thus, if the user presses Reload and the node's edit page is loaded _before_ the invalid ajax unlock request completes, the ajax unlock request will be rejected because it has the old ajax_key, not the new one.

Thus, normal lock breaking by navigation away from a page still works and page refreshes can no longer break locks -- even without trying to detect the page refresh and avoid sending the ajax unlock request. However, this _does_ break ajax unlocking of a loaded node edit page when the user opens the same node edit page in a new tab. Only the most recently opened or reloaded tab will have the ability to remove locks upon the user browsing away. (But hopefully a person using multiple tabs will know what he's doing so that we needn't worry about this). In this latter case, where a user opens a node edit page in one tab, then in another tab, and then returns to using the first tab, the user will still be bugged by content_lock's ``you still have a node locked'' message after he browses away from the node. That should be enough.

ohnobinki’s picture

ohnobinki commented

The code is detecting the reload fine.

I'm pretty sure that if the reload was properly detected, an AJAX request to unlock the node wouldn't be sent. That code should only be invoked when the user is browsing away. But I'm quite sure there's no proper way to detect this ``the user wanted to reload the page'' case.

The problem is that Firefox first sends the request for the new page to the server (which keeps the lock), and then fires the handler on the old page, which releases the lock.

That explains all of the behavior; I wrote my fix above with that in mind. Thanks! :-)

Bodo Maass’s picture

Bodo Maass commented

I guess you are right in saying that the code is not detecting when the user clicks the reload button. It traps the keys that trigger a reload, but it fails to detect when the user clicks on the reload button with the mouse (and as far as I have found out so far that is not even possible in most browsers).

Anyway, your fix sounds great. I'll give it a try.

ohnobinki’s picture

ohnobinki commented
Status: Needs review » Fixed

In content_lock-6.x-2.5.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.