Modify

Opened 12 years ago

Closed 11 years ago

#10276 closed enhancement (fixed)

"Unknown preference panel" when logging out from account tab

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

Description (last modified by Ryan J Ollos)

  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 ; Changed 12 years ago by Steffen Hoffmann

Keywords: logout redirect added
Severity: normalminor
Type: defectenhancement

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 ; Changed 12 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 ; Changed 12 years ago by Steffen Hoffmann

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 12 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 12 years ago by Steffen Hoffmann

(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 Changed 12 years ago by Steffen Hoffmann

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 12 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 12 years ago by Ryan J Ollos

Description: modified (diff)

comment:9 Changed 11 years ago by Steffen Hoffmann

Resolution: fixed
Status: newclosed

(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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.