Revo 2.2.11 annoying "leave this page" message

Quote from: rich at Jan 24, 2014, 06:11 AM

@pauld123

Are you talking about this fix?

/*  e.browserEvent.returnValue = ''; */

I can't find this line in my manager\assets\modext\sections\resource\update.js file. Are you running MODX 2.2.11?

Actually I am running 2.2.10-pl (advanced). Not sure I want to upgrade just yet though.

And yes, that is exactly the line that once removed fixed those annoying messages. Although I had put up with them for some time they seemed to be getting worse, appearing more and more often - which might just be me getting more and more annoyed with them. Now they are gone am back to 100% pleasure using the manager.

I just installed on a new site, but when I clicked (for the first time) toggle messages I just go a javascript void error.

Just thought I would mention that although that might be a 2.2.10 issue resolved for 2.2.11

Although I consider myself a fair programmer, looking through the core files for MODX am completely baffled, so there is no chance I am going to be able to help with fixing anything - sorry.

Paul.

PS I so love the installation process, so clean and easy. Even the 'delete installation files' at the end is such a lovely touch. I think only wordpress does it a tad slicker, but colder, other than that nothing comes close in my experience. Nothing to do with this thread but just thought I would mention it

On 2.2.11-pl, if you get annoyed by the warning, just comment those lines https://github.com/modxcms/revolution/blob/release-2.2/manager/assets/modext/widgets/resource/modx.panel.resource.js#L66-69

You will loose a really nice feature (which might need some more tests to behave "as expected" -- or find the culprit making it not behaving correctly).

Hope this will help.

Quote from: romain at Jan 25, 2014, 07:58 AM

On 2.2.11-pl, if you get annoyed by the warning, just comment those lines https://github.com/modxcms/revolution/blob/release-2.2/manager/assets/modext/widgets/resource/modx.panel.resource.js#L66-69

You will loose a really nice feature (which might need some more tests to behave "as expected" -- or find the culprit making it not behaving correctly).

Hope this will help.

Thanks, I will try it as soon as I'm back in office!
Actually it could be a nice feature IF it pop-ups ONLY when I make actual changes to the fields, not just when I open a resource (maybe just to see if a value is ok) and then move to another. ;P

I'd say this is the culprit

Don't know enough about those to know exactly what is wrong, though. There should be a way to indicate "!this.change.template" or something like that.

And the Save button is not reliable enough to use as a trigger, in my opinion. It's often not enabled but you can click it anyway, or doesn't become disabled after a save.

Quote from: sottwell at Jan 25, 2014, 08:07 AM

I'd say this is the culprit

      if ((this.config.record && this.config.record.richtext) || MODx.request.reload || MODx.request.activeSave == 1) {
                this.markDirty();
            }

Don't know enough about those to know exactly what is wrong, though. There should be a way to indicate "!this.change.template" or something like that.

And the Save button is not reliable enough to use as a trigger, in my opinion. It's often not enabled but you can click it anyway, or doesn't become disabled after a save.

You're right. As soon as I click a resource in the tree to view it, the Save button is active. Also, it looks like this script makes triggers the notice if the screen is reloaded, regardless of whether anything else has happened.

A patch for the issue that started appearing in 2.2.11 is now available: https://github.com/modxcms/revolution/pull/820


Quote from: sottwell at Jan 25, 2014, 08:07 AM

I'd say this is the culprit

      if ((this.config.record && this.config.record.richtext) || MODx.request.reload || MODx.request.activeSave == 1) {
                this.markDirty();
            }

Don't know enough about those to know exactly what is wrong, though. There should be a way to indicate "!this.change.template" or something like that.

And the Save button is not reliable enough to use as a trigger, in my opinion. It's often not enabled but you can click it anyway, or doesn't become disabled after a save.

I could not find a reason why it would be checking for richtext there, but it was not the source of the bug and supposedly is in there for a reason... The source of the bug is the "isDirty" method of the FormPanel, combined with rich text editors. Basically RTEs like to clean up content (wrap it in paragraphs, encode entities etc) which makes the content different from the original, marking the form dirty while the user made no changes, and boom: navigation warning. The markDirty method has no influence on the isDirty one, ironically. Nitty gritty of the bug here.

The patch changes the logic to actually wait for fields to trigger a change event and only triggers the warning if that has happened. This resolves issues on create, reloading etc.

Quote from: markh at Jan 26, 2014, 08:11 AM

A patch for the issue that started appearing in 2.2.11 is now available: https://github.com/modxcms/revolution/pull/820

I could not find a reason why it would be checking for richtext there, but it was not the source of the bug and supposedly is in there for a reason... The source of the bug is the "isDirty" method of the FormPanel, combined with rich text editors. Basically RTEs like to clean up content (wrap it in paragraphs, encode entities etc) which makes the content different from the original, marking the form dirty while the user made no changes, and boom: navigation warning. The markDirty method has no influence on the isDirty one, ironically. Nitty gritty of the bug here.

The patch changes the logic to actually wait for fields to trigger a change event and only triggers the warning if that has happened. This resolves issues on create, reloading etc.

I applied all these changes, cleared all MODX and browser caches, and the problem persists. Every time I click a different resource in the tree, the alert shows up. I'm even getting the alert if I click on a resource that is a weblink, so there is no rich text field, and then click on a different resource in the tree. I am also still getting the alert if I save a resource and then reload the page. I am NOT getting the message when changing resource type or template, so that is fixed. I've tested on Mac Firefox, Safari, and Chrome.

It's nice having the system setting to turn this on and off, though. That is working well.

Could you PM me a login to your site? If not no problem, but I've been testing it most of the afternoon in Chrome and Firefox and it was behaving exactly as it should since the patch was done. Would like to see with my own eyes that the patch is applied properly and that we're talking about the same issue.

Quote from: rainbowtiger at Jan 25, 2014, 12:50 PM

You're right. As soon as I click a resource in the tree to view it, the Save button is active. Also, it looks like this script makes triggers the notice if the screen is reloaded, regardless of whether anything else has happened.

The save button shouldn't be active as soon as a resource is activated, only when something has changed, so it seems that's where your problem is at. What's causing that will be interesting to see.

Thanks @rainbowtiger.

You're using CacheMaster on that site, which unsets the "Empty Cache" field on the Settings tab to prevent the cache from automatically being cleared on saving a resource. While it's not something you as user are consciously doing, it is definitely a change in the form that the code should be hooking into - and explains why the save button instantly enables.

I could build in an exception that if the "empty cache" checkbox is toggled it does not affect the navigation confirmation stuff.. in the case of that specific checkbox the value is not actually stored in the database, but it only affects the save processing. So technically speaking, it is not unsaved data as it wouldn't be saved if you hit save anyway.

EDIT: Done. Commit https://github.com/Mark-H/revolution/commit/7e95477189787071f11f9a7c2a06969df6b76b90 part of the original PR here https://github.com/modxcms/revolution/pull/820 Also applied to your site rainbowtiger. [ed. note: markh last edited this post 12 years, 8 months ago.]

Quote from: markh at Jan 26, 2014, 11:08 AM

Thanks @rainbowtiger.

You're using CacheMaster on that site, which unsets the "Empty Cache" field on the Settings tab to prevent the cache from automatically being cleared on saving a resource. While it's not something you as user are consciously doing, it is definitely a change in the form that the code should be hooking into - and explains why the save button instantly enables.

I could build in an exception that if the "empty cache" checkbox is toggled it does not affect the navigation confirmation stuff.. in the case of that specific checkbox the value is not actually stored in the database, but it only affects the save processing. So technically speaking, it is not unsaved data as it wouldn't be saved if you hit save anyway.

EDIT: Done. Commit https://github.com/Mark-H/revolution/commit/7e95477189787071f11f9a7c2a06969df6b76b90 part of the original PR here https://github.com/modxcms/revolution/pull/820 Also applied to your site rainbowtiger.

Thanks, it works great now! Interesting about Cachemaster. And yes, Empty Cache is the only checkbox that isn't actually data that gets saved, so it makes sense to ignore how that is set in terms of triggering the alert.