Closed Bug 1492154 Opened Last year Closed Last year

Session service does not handle custom protocols properly

Categories

(SeaMonkey :: Session Restore, defect)

SeaMonkey 2.49 Branch
defect
Not set

Tracking

(seamonkey2.49esr fixed, seamonkey2.53 affected, seamonkey2.57esr fixed, seamonkey2.60 wontfix)

RESOLVED FIXED
Future
Tracking Status
seamonkey2.49esr --- fixed
seamonkey2.53 --- affected
seamonkey2.57esr --- fixed
seamonkey2.60 --- wontfix

People

(Reporter: Off.Just.Off, Assigned: frg)

References

Details

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20100101

Steps to reproduce:

1. Set up the browser to restore previous session on startup
2. Install Classic Add-ons Archive (CAA) add-on from https://github.com/JustOff/ca-archive/releases/download/1.1.4/ca-archive-1.1.5b1.xpi
3. Click the CAA button to open the catalog (or type "caa:" in the urlbar)
4. Restart the browser


Actual results:

1. The console displays an error at https://dxr.mozilla.org/comm-esr52/source/suite/common/src/nsSessionStore.js#1696 when you explore CAA.
2. The tab with CAA does not restored after the browser restart.



Expected results:

All tabs must be restored and errors should not occur.
I think this can be easily fixed by replacing

  let origin = principal.origin;

with

  let origin;
  try {
    origin = principal.origin;
  }
  catch (ex) {
    origin = principal.URI.spec;
  }

at https://dxr.mozilla.org/comm-esr52/source/suite/common/src/nsSessionStore.js#1696

PS: I would also like to draw your attention that the condition at https://dxr.mozilla.org/comm-esr52/source/suite/common/src/nsSessionStore.js#1700 will never be fulfilled, as the principal does not contain either uri or url.
Depends on: 776577
Status: UNCONFIRMED → NEW
Ever confirmed: true
> origin = principal.origin;

You sure the assignnemt throws and not just returns null in which case and if (!... might be better?

> as the principal does not contain either uri or url.
Hmm this looks like a typo. Should be URI I think. 6 years old. Must not be important :)

I look at it when I find some time. Maybe this weekend.
> You sure the assignment throws and not just returns null?

NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIPrincipal.origin]

> I look at it when I find some time.

Thank you.
Attached patch 1492154-sessionstore-249.patch (obsolete) — Splinter Review
Changes seem to not cause a regression but I didin't try the build yet much.

I will send the patch to Bill for putting into his unoffical builds:

http://www.wg9s.com/comm-esr/

Please check if it fixes you regression when the build is up. Patch should appear in this directory then:
http://www.wg9s.com/comm-esr/patches/git/comm-esr52/

Porting to 2.53+ now.
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
Flags: needinfo?(Off.Just.Off)
just the comment changed.
Attachment #9011217 - Attachment is obsolete: true
Patch for 2.53 and 2.57
Thank you, I confirm the regression is fixed in WG9s SeaMonkey 2.49.5 build 20180923041539.
Flags: needinfo?(Off.Just.Off)
Comment on attachment 9011276 [details] [diff] [review]
1492154-sessionstore.patch

Into all trees for the Good of SeaMonkeykind
Attachment #9011276 - Flags: review?(iann_bugzilla)
Attachment #9011276 - Flags: approval-comm-esr60?
Attachment #9011276 - Flags: approval-comm-esr52?
Comment on attachment 9011276 [details] [diff] [review]
1492154-sessionstore.patch

Wonder how those typos slipped through
LGTM r/a=me
Attachment #9011276 - Flags: review?(iann_bugzilla)
Attachment #9011276 - Flags: review+
Attachment #9011276 - Flags: approval-comm-esr60?
Attachment #9011276 - Flags: approval-comm-esr60+
Attachment #9011276 - Flags: approval-comm-esr52?
Attachment #9011276 - Flags: approval-comm-esr52+
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/bb81266621e3
Allow for invalid origin in principal and fix misspelled principal uri and url in sessionstore. r=IanN
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.