Modify

Opened 2 years ago

Closed 21 months ago

#10276 closed enhancement (fixed)

"Unknown preference panel" when logging out from account tab

Reported by: skostysh@… Owned by: hasienda
Priority: normal Component: AccountManagerPlugin
Severity: minor Keywords: logout redirect
Cc: Trac Release: 0.12

Description (last modified by rjollos)

  1. go to http://www.lyx.org/trac/
  2. login
  3. click on "account"
  4. logout

When I do this on Firefox or Chromium on Ubuntu I get "Error: Not
Found. Unknown preference panel"
I think this is because trac (AccountManager?) tries to allow the user to change
preferences even if the user is not registered. Thus, when one logs
out, it tries to put the user on the anonymous preferences page. But "account" does not exist for anonymous preferences.

Thanks,

Scott

Attachments (0)

Change History (9)

comment:1 in reply to: ↑ description ; follow-up: Changed 2 years ago by hasienda

  • Keywords logout redirect added
  • Severity changed from normal to minor
  • Type changed from defect to enhancement

Replying to skostysh@lyx.org:
...

When I do this on Firefox or Chromium on Ubuntu I get "Error: Not
Found. Unknown preference panel"

I see, what you're pointing at, ok.

I think this is because trac (AccountManager?) tries to allow the user to change
preferences even if the user is not registered. Thus, when one logs
out, it tries to put the user on the anonymous preferences page. But "account" does not exist for anonymous preferences.

AcctMgr does nothing more or less than trying to not interfere by redirecting right back to the same page, as if you didn't went away from the page.

Not sure, if you understand the beauty of this approach, or how hard it was to do so - certainly not simple. In your case it's a bit different, because unlike 'ticket/..' or '/wiki/..' the '/prefs' path and it's descendants have a different meaning now depending on context - the session ID here.

Well, there we are. Trac goes for a non-existent page after the context change, and as a consequence it throws a crystal clear error back on you.

Something wrong with that? At least you seem to dislike this behavior, you claim or suggest an AcctMgr defect - but that's it. Come on, you clearly fail to tell us about your expectations.

Wait, before you do, think twice, because I'm sure, there are dozens of "correct" answers, depending largely on personal preferences. If you know a sufficiently generic approach, please speak out loud. If you can provide some real code - a patch would be 50 % towards improving the situation. Only, this is bound to be an enhancement request, or you tell me about an AcctMgr defect, that I failed to read between your lines.

Anyway, thanks for taking care. Feedback is welcome, and the only way to make it better tomorrow.

comment:2 in reply to: ↑ 1 ; follow-up: Changed 2 years ago by skostysh@…

Replying to hasienda:

Replying to skostysh@lyx.org:
AcctMgr does nothing more or less than trying to not interfere by redirecting right back to the same page, as if you didn't went away from the page.

Not sure, if you understand the beauty of this approach, or how hard it was to do so - certainly not simple. In your case it's a bit different, because unlike 'ticket/..' or '/wiki/..' the '/prefs' path and it's descendants have a different meaning now depending on context - the session ID here.

I think I did understand the beauty of it but I did not understand how difficult it was to implement. I wonder if that's a sign of good coding -- when someone thinks something is easy even though it was hard to implement. Kind of like how professional sports players can make crazy things look almost simple. In any case, good work coding this up and thank you.

Well, there we are. Trac goes for a non-existent page after the context change, and as a consequence it throws a crystal clear error back on you.

Something wrong with that? At least you seem to dislike this behavior, you claim or suggest an AcctMgr defect - but that's it. Come on, you clearly fail to tell us about your expectations.

Wait, before you do, think twice, because I'm sure, there are dozens of "correct" answers, depending largely on personal preferences. If you know a sufficiently generic approach, please speak out loud.

You are right that I did not give my expectation. This was on purpose. I do not know much about trac and figured that telling you my expecations in this case would be pretentious -- kind of like I know what trac should do more than you do. Because there are dozens of correct answers I figured that the developers were the best placed to know which one is most correct. I think it is important to give your expectation when it's not obvious that there is a problem. To me the message "Error" makes it obvious that there is a problem. However, as you suggest, this might be because of my lack of knowledge of trac and the underlying code.

Anyway, thanks for taking care. Feedback is welcome, and the only way to make it better tomorrow.

Great, and thank *you*! I really appreciate the quick reply and the pride that you take in the work that you do.

Scott

comment:3 in reply to: ↑ 2 ; follow-up: Changed 2 years ago by hasienda

Replying to skostysh@lyx.org:

Replying to hasienda:

Replying to skostysh@lyx.org:
AcctMgr does nothing more or less than trying to not interfere by redirecting right back to the same page, as if you didn't went away from the page.

...

I figured that the developers were the best placed to know which one is most correct. I think it is important to give your expectation when it's not obvious that there is a problem. To me the message "Error" makes it obvious that there is a problem. However, as you suggest, this might be because of my lack of knowledge of trac and the underlying code.

Anyway, thanks for taking care. Feedback is welcome, and the only way to make it better tomorrow.

Great, and thank *you*! I really appreciate the quick reply and the pride that you take in the work that you do.

Ok, I take this for granted anyway. Most of us do it non-profit, many are "standing on the shoulders of giants", saying there is much code that is not my own, nor are ideas I copied elsewhere. But you're right, that doing it with pride has it's own reward in the work itself. But stop that now, it's philosophy.

You have earned a point on the error thing. It should not error out when doing right, or foreseeable wrong. After all AcctMgr provides this "account" preference sub-page, so it's the best bet, that it chould care for it in any situation. As a matter of fact any other Trac part certainly does not even know neither about AcctMgr nor specifically about 'prefs/account'.

Let me think about it, and please nudge me again, if you feel it's taking too long. I'm doing some totally unrelated work - for new user registration - right now, but it seems not too hard to find a way for prevent the error. Most probably we'll need a default redirect to kick-in whenever 'prefs/account' is hitten by anonymous users. Appreciate the hint on this.

comment:4 in reply to: ↑ 3 Changed 2 years ago by skostysh@…

Replying to hasienda:

Replying to skostysh@lyx.org:

Replying to hasienda:

Replying to skostysh@lyx.org:
AcctMgr does nothing more or less than trying to not interfere by redirecting right back to the same page, as if you didn't went away from the page.

...

I figured that the developers were the best placed to know which one is most correct. I think it is important to give your expectation when it's not obvious that there is a problem. To me the message "Error" makes it obvious that there is a problem. However, as you suggest, this might be because of my lack of knowledge of trac and the underlying code.

Anyway, thanks for taking care. Feedback is welcome, and the only way to make it better tomorrow.

Great, and thank *you*! I really appreciate the quick reply and the pride that you take in the work that you do.

Ok, I take this for granted anyway. Most of us do it non-profit, many are "standing on the shoulders of giants", saying there is much code that is not my own, nor are ideas I copied elsewhere. But you're right, that doing it with pride has it's own reward in the work itself. But stop that now, it's philosophy.

True, this is a conversation best had over a bottle of wine :)

You have earned a point on the error thing. It should not error out when doing right, or foreseeable wrong. After all AcctMgr provides this "account" preference sub-page, so it's the best bet, that it chould care for it in any situation. As a matter of fact any other Trac part certainly does not even know neither about AcctMgr nor specifically about 'prefs/account'.

Let me think about it, and please nudge me again, if you feel it's taking too long. I'm doing some totally unrelated work - for new user registration - right now, but it seems not too hard to find a way for prevent the error. Most probably we'll need a default redirect to kick-in whenever 'prefs/account' is hitten by anonymous users. Appreciate the hint on this.

OK sounds good. Thanks again for such quick replies and explanations!

Scott

comment:5 Changed 2 years ago by hasienda

(In [11938]) AccountManagerPlugin: Avoid a TracError on logout, refs #10276.

Logging out from 'account' session/user preferences is a forseeable operation,
so it should better not raise an error. Thanks to Scott for reporting it.
Now, redirect unauthenticated requests to general preferences should be safe.
I've removed some code, that was meant to cope with this situation beforehand,
but it seems to never be reached before Trac decided to error out.

comment:6 follow-up: Changed 2 years ago by hasienda

Feedback appreciated.

Note: Please don't close issues here. I'll do so with next release, that incorporates the relevant code, if the issue seems resolved, or I'll try harder to fix it better next time.

comment:7 in reply to: ↑ 6 Changed 2 years ago by skostysh@…

Replying to hasienda:

Feedback appreciated.

Note: Please don't close issues here. I'll do so with next release, that incorporates the relevant code, if the issue seems resolved, or I'll try harder to fix it better next time.

OK. This is definitely the quickest that a bug/feature I've reported has been resolved.

Thanks a lot, hasienda!

Scott

comment:8 Changed 2 years ago by rjollos

  • Description modified (diff)

comment:9 Changed 21 months ago by hasienda

  • Resolution set to fixed
  • Status changed from new to closed

(In [12398]) AccountManagerPlugin: Releasing version 0.4, pushing development to acct_mgr-0.5dev.

Availability of that code as stable release
closes #874, #3459, #4677, #5295, #5691, #6616, #7577, #8076, #8685, #8770, #8791, #8990, #9052, #9079, #9090, #9139, #9246, #9252, #9547, #9618, #9676, #9843, #9852, #9940, #10023, #10028, #10123, #10142, #10204, #10276, #10397, #10412, #10594, #10625 and #10644.

Some more issues have been worked-on, yet without confirmed resolution,
refs #5464 (for JiraToTracIntegration), #8927 and #10134.

And finally there are some issues and enhancement requests showing progress,
but known to require more work to resolve them satisfactorily,
refs #843, #1600, #5964, #8217, #8933.

Thanks to all contributors and followers, that enabled and encouraged a good
portion of this development work.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.