Ticket preferences are not saved

[Ryan J Ollos]

Reported in ​trac:#11612 and ​trac:#11808, the ticket preferences comment sort order and comments only are not saved from an authenticated session. This is due to Registration.pre_process_request() intercepting the AJAX post and redirecting.

Here are the comments that were originally posted to ​trac:#11612:

---

I can reproduce the issue in this ticket as well as ​trac:#11808 when AccountManagerPlugin is installed. Setting a breakpoint at the location the ​AJAX callback is processed, I see thatreq.method == 'GET' when AccountManagerPlugin is enabled, which is the direct cause for the preference not being saved.

As to the reason why this is a GET request rather than a POST, the only guess I have at the moment is that another handler in account manager matches the request and redirects, converting the ​jQuery AJAX POST request to a GET. I'll debug further when I have more time tomorrow.

---

The problem is in EmailVerificationModule.pre_process_request(). When the user is authenticated the account validation is executed in response to an AJAX POST, which raises a RegistrationError exception with message You must specify a valid email address (which Jun noted as seeing in ​trac:comment:7:ticket:11808): accountmanagerplugin/trunk/acct_mgr/register.py@14078:551,566#L542​.

The following patch at least works around the issue:

accountmanagerplugin/trunk/acct_mgr/register.py

@@ -546 +546 @@
             # Permissions for anonymous users remain unchanged.
             return handler
         elif req.path_info == '/prefs' and req.method == 'POST' and \
-                not 'restore' in req.args:
+                not 'restore' in req.args and \
+                not req.get_header('X-Requested-With') == 'XMLHttpRequest':
             try:
                 AccountManager(self.env).validate_account(req)
                 # Check passed without error: New email address seems good.

[Steffen Hoffmann]

Too bad there is no dedicated URL for such preferences, so I see no chance to avoid such a rather ugly condition refinement too.

[Ryan J Ollos]

I'll keep that in mind while working ​trac:#11612. We might be able to improve things on the trunk.

[Steffen Hoffmann]

I'll do the fix here.

Replying to rjollos:

I'll keep that in mind while working ​trac:#11612. We might be able to improve things on the trunk.

Yes. Neither these settings nor other new general (new) ones should be sent to /pref, to avoid such side-effects. I remember having seen a dedicated path for AJAX requests in DiscussionPlugin; could be worth adopting that pattern for clarity.

[Steffen Hoffmann]

In 14272:

AccountManagerPlugin: Ignore AJAX POST requests from Trac core, refs #12050.

User preferences for activating alternative ticket change history
presentations have been added in Trac 1.0.

Filtering requests to '/prefs' more strictly in EmailVerificationModule
works around the reported failure of saving these new preferences, if that
account manager module is enabled.

In the long run it would be good to keep request to Trac URLs like '/admin' or
'/prefs' more backwards-compatible.