In bug 1331686 we added code that serializes principals. The patch initially used the sessionstore utils which wrap the serialization helper with a method called "serializePrincipal". Unfortunately, the final checkin continues to use that method name but calls it on the serialization helper, where it doesn't exist. I should probably have noticed this in review. :-(

(In reply to :Gijs from comment #0) > In bug 1331686 we added code that serializes principals. The patch initially > used the sessionstore utils which wrap the serialization helper with a > method called "serializePrincipal". Unfortunately, the final checkin > continues to use that method name but calls it on the serialization helper, > where it doesn't exist. I should probably have noticed this in review. :-( Checking the comments on bug 1331686, my initial review feedback mistakenly referred to: > and then in here use gSerializationHelper.serializePrincipal instead. Sorry. :-(

Comment on attachment 8830237 [details] Bug 1333726 - use serializeToString instead of non-existant serializePrincipal, https://reviewboard.mozilla.org/r/107132/#review108224 r=me, thanks for fixing. This needs uplifting too.

(In reply to :Gijs from comment #2) > > and then in here use gSerializationHelper.serializePrincipal instead. > > Sorry. :-( Yeah, no worries, I could/should have checked myself. Interestingly is that no test triggered that codepath.

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/03cc3fbf7ae3 use serializeToString instead of non-existant serializePrincipal, r=ckerschb

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #4) > Yeah, no worries, I could/should have checked myself. Interestingly is that > no test triggered that codepath. Yes. Filed bug 1333727

(In reply to :Gijs from comment #6) > (In reply to Christoph Kerschbaumer [:ckerschb] from comment #4) > > Yeah, no worries, I could/should have checked myself. Interestingly is that > > no test triggered that codepath. > > Yes. Filed bug 1333727 Awesome - thank you!

Comment on attachment 8830237 [details] Bug 1333726 - use serializeToString instead of non-existant serializePrincipal, Approval Request Comment [Feature/Bug causing the regression]: bug 1331686 [User impact if declined]: some loads fail [Is this code covered by automated tests?]: in general, yes. This specific failure, apparently not. We'll look at tests in bug 1333727, but this is such a trivial change that I think it's worth landing on aurora even before the tests are done. [Has the fix been verified in Nightly?]: not yet [Needs manual test from QE? If yes, steps to reproduce]: I don't think so, the change is too trivial to bother (please see patch and other bug comments) [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: simple change to correct a method name we're calling [String changes made/needed]: no

Comment on attachment 8830237 [details] Bug 1333726 - use serializeToString instead of non-existant serializePrincipal, Should fix some user issues with page loading, let's uplift for 53.