Top "Save" button doesn't work!

[mindplay-dk]

Looks like the latest release version has a pretty nasty bug: the "Save" button at the top of the page does not actually save anything!

As it turns out, the "Save" button at the top of the page is located outside the form it's supposed to submit - hence it's value is not being submitted, which, when you examine the logic inside the controller, means nothing gets saved.

What's worse - the code that outputs confirmation messages saying "body saved" etc. are being executed, as they're located outside the if-statement that governs whether anything actually gets saved.

Took me almost half a day of chasing my own tail to pinpoint this bug :-(

I wonder how many people have lost important changes by pressing that button, thinking everything was just fine when the admin page reports back "this saved, that saved!".

Apparently not many, or you probably would have heard about it. Which makes me wonder if anybody uses the top-most "Save" button at all?

[adrianbj]

Two relevant posts: http://processwire.com/talk/topic/573-top-save-page-button-dont-work/ http://processwire.com/talk/topic/3242-top-save-button-doesn%C2%B4t-work-in-ie9/

[mindplay-dk]

I guess I don't understand how this was expected to work, or why it would work in the first-place - the button form-element is not actually nested in the form being submitted, hence the correct/expected browser-behavior is that it's value should not get submitted.

A form-associated element might work in newer browsers, but some JavaScript hocus-pocus would be required to make this work in older browsers.

PS: I'm using the latest tagged PW release and latest beta-release of Chrome.

[mindplay-dk]

This bug is critical and really needs to be fixed urgently - users are losing content!

For now I hot-patched main.css on all of our PW projects:

button#submit_save_copy { display:none; }

(I highly recommend you do the same or warn your clients about this issue until this gets fixed.)

[ryancramerdesign]

I've not been able to duplicate this one. I probably use that button a couple hundred times a day, but it always saves as it should. And if it were really broken, I don't think anyone would be successfully using PW to manage content. :) Meaning, we'd be hearing about it several times a day. The only place where I know it doesn't work is in some versions of IE, but we have code disabling the button for those versions of IE (though I've not personally tested in all versions of IE). You already mentioned you are using Chrome (which is also what I'm using), but is it possible there is some issue with it being a Beta release of chrome? (I'm on stable rather than beta for Chrome). Also, what admin theme are you using (if not the default)? Are you running any other modules that might be interfering?

[mindplay-dk]

I suspected an issue with the admin theme, but I did already remove that and was able to reproduce the bug.

I have a few modules of my own, and also suspected those, but was able to reproduce the bug with all custom modules disabled too.

I am using the latest tagged release, not the dev/master.

[apeisa]

I was wondering this issue, since we have about 20 sites running various versions of PW and none had this issue reported. So I went ahead and downloaded the latest tagged release (2.3.0) from here https://github.com/ryancramerdesign/ProcessWire/releases and made fresh install.

I was unable to reproduce this problem. I tried all kinds of actions: creating new pages and publishing from top button, editing existing pages etc.. All worked fine. I tried latest FF and Chrome. I tried IE too, but even on IE 10 the top button is not visible).

So the reason most be somewhere else: custom admin theme or modules, some strange browser issue or server enviroment (like mod_security or something like that).

[mindplay-dk]

Interesting... most likely a browser issue then, because I was able to reproduce this on at least 5 different PW installations, all different versions, some dev/master and some release-versions...

Could I convince one of you to switch to the beta channel of Chrome and re-test? (You can always switch back.)

There's a good chance whatever is broken in the beta-release will soon be broken in the stable release, as the beta-channel in the "last stop" before new features are released to the general audience...

[apeisa]

Changed to chrome beta channel, still not able to reproduce. Have you tried different browsers on your end or is this Chrome specific?

Can you provide test credentials on any test site you are having this issue? I am happy to provide you access to my testsite (just contact me at antti.peisa - gmail) so you can try if it is still a browser issue.

[mindplay-dk]

Just re-tested with Firefox and Chrome - it only happens in Chrome.

Inspecting the raw HTTP POST (with Fiddler) and comparing between Firefox and Chrome, the only notable difference is the missing "submit_save" form-data section in Chrome's post, which is consistent with what I narrowed it down to in the code.

I haven't looked into it, but since the <button> isn't nested in the <form> tag, presumably there must be a JavaScript dependency, something that is supposed to make this work?

I kept the console open (with preserve on navigation) and there are no JavaScript-errors, which could mean that either the script isn't executing, or it doesn't do what it's supposed to.

If you can point me to that script, I can take a closer look?

[apeisa]

The code that clones the submit button and triggers the submit is on /templates-admin/scripts/main.js.

https://github.com/ryancramerdesign/ProcessWire/blob/master/wire/templates-admin/scripts/main.js#L53

[apeisa]

Earlier it submitted the form (that is when I guess it might be possible what you are describing, that some browsers leave the submit button value out of the POST), but on latest stable (also the tagged version) it just triggers the click action of the real submit button.

[mindplay-dk]

Well, this is odd... as of this morning, I can no longer reproduce the bug with the standard admin-theme.

Is it possible the previous version (from before I upgraded PW to the latest tagged release) was still cached on the client somehow? I noticed PW uses URLs with version-numbers in them, e.g. file.js?v=123 which isn't really the de-facto standard way to control client-side caching.

How about using an ETag instead? That might be more reliable. It's fairly common for developers to forget to increment the version-number with every minor change to any file in every module.

The bug is still present in the custom admin-theme I'm using, so on another related note - is it good/common practice for admin-themes to clone and modify the JavaScript dependencies? Perhaps if the code was more modular and didn't initialize itself directly ($.ready(...)) there would be less of a tendency for admin-theme developers to clone the JS, which is a maintenance burden and apparently a real problem when that maintenance doesn't happen.

Standardizing on minimal AMD might simplify things for developers, and also opens up a potential path to optimize/minify script-dependencies in the future? (I'm not personally a fan of full-blown AMD which seems like a big, complicated solution waiting for a problem - but I have found minimal AMD to be very worth while and much easier to understand...)

[tbba]

Just a note: A.f.a.i.k. there was a change within this current dev cycle of PW, how that button works. Not all admin-templates got updates accordingly to reflect these changes, if I see this correctly. If you use the popular TEFLON as admin theme by chance, you might want to try the TEFLON2 beta (from GitHub) where this button issue was addressed. At least for my clients this admin theme version works flawlessly.

[mindplay-dk]

I use a customized fork of Teflon which could probably use an update at this point...

[ryancramerdesign]

Automatically closing out old issues, but not looking too closely, so please re-open if you find the issue to still be a problem. Thanks, –Ryan