Closed
Bug 665708
Opened 13 years ago
Closed 10 years ago
Gopher links show "Launch Application" in SeaMonkey 2.1
Categories
(SeaMonkey :: Download & File Handling, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.28
People
(Reporter: dsmutil, Assigned: ewong)
Details
Attachments
(2 files, 5 obsolete files)
5.85 KB,
image/png
|
Details | |
11.75 KB,
patch
|
ewong
:
review+
|
Details | Diff | Splinter Review |
When I follow one of my gopher links, I now get an error from SeaMonkey: Launch Application Send to: SeaMonkey Choose an Application I select SeaMonkey but it just keeps coming up again. I found some bugs that mention that Gopher may be removed in a future version of SeaMonkey but the error page is unclear if it has been removed. A group of us just started creating gopher pages about a year ago and now it's broken.
Gopher support has been removed from SM and FF (Bug 388195). You might want to try Overbite (https://addons.mozilla.org/en-US/seamonkey/addon/overbiteff/).
Comment 2•13 years ago
|
||
I agree this could be clearer. We didn't take Bug 572000 in the end (see Bug 572000 comment 52) but if someone were to adapt the patch in that bug to SeaMonkey and submit it here, we would certainly consider it.
Reporter | ||
Comment 3•13 years ago
|
||
Thanks. I put a note over on bug 572000 asking for help. Should something be put in the release notes? Or, could Overbite be bundled with the installation, such as ChatZilla, DOM Inspector, and the JavaScript Debugger, so it could be installed with a custom install?
Comment 4•13 years ago
|
||
No plans to bundle OverbiteFF with SeaMonkey. Bundling is already too complicated as it is.
Comment 5•13 years ago
|
||
Philip, I'd be willing to port if you're willing to review/accept.
Assignee | ||
Comment 6•10 years ago
|
||
Hi Cameron.. just wondering if you'll be doing the patch. It wouldn't be right if I took that patch from that other bug and adapt it to suite/.
Comment 7•10 years ago
|
||
Just to clarify, we are thinking of taking the Gopher stub protocol handler patch in Bug 572000 and adapting it for SeaMonkey. Since this will be in comm-central/suite/ we won't have to get any of the Core/networking people to sign off on it.
Flags: needinfo?(spectre)
Comment 8•10 years ago
|
||
I'm fine with whatever. I don't have a build of SeaMonkey up right now, so I'm not in a position to test this easily. If you can do so, Edmund, go for it.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → ewong
Status: NEW → ASSIGNED
Assignee | ||
Updated•10 years ago
|
Version: SeaMonkey 2.1 Branch → Trunk
Assignee | ||
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8350864 -
Attachment is obsolete: true
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8350865 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Version: Trunk → SeaMonkey 2.1 Branch
Assignee | ||
Updated•10 years ago
|
Attachment #8350866 -
Flags: review?(neil)
Comment 13•10 years ago
|
||
Comment on attachment 8350866 [details] [diff] [review] proposed patch (v1) >diff --git a/suite/common/gopher/Makefile.in b/suite/common/gopher/Makefile.in This file is unnecessary. >diff --git a/suite/common/gopher/gopherAddon.xhtml b/suite/common/gopher/gopherAddon.xhtml This doesn't deserve its own directory, just put it in suite/common. (Move the .dtd too and update jar.mn and newChannel etc.) >+ <link rel="stylesheet" href="chrome://global/skin/netError.css" type="text/css" media="all" /> Nit: don't need the media attribute, or the space before /> >+ document.location = "http://addon.mozilla.org/addon/7685"; Nit: addons.mozilla.org Hmm, I wonder whether we're safe to hard-code the url here. >diff --git a/suite/common/gopher/moz.build b/suite/common/gopher/moz.build This file is unnecessary. >+ >+ Nit: doubled blank line >+ classDescription : "Gopher protocol handler stub", Nit: no space before : please >+ contractID : "@mozilla.org/network/protocol;1?name=gopher", Nit: contactID no longer necessary >+ // nsISupports >+ QueryInterface : XPCOMUtils.generateQI([Components.interfaces.nsISupports, >+ Components.interfaces.nsIProtocolHandler]), XPCOMUtils.generateQI already, um, supports, nsISupports. >+ protocolFlags: Components.interfaces.nsIProtocolHandler.URI_NORELATIVE >+ | Components.interfaces.nsIProtocolHandler.URI_NOAUTH >+ | Components.interfaces.nsIProtocolHandler.URI_LOADABLE_BY_ANYONE, Nit: | belongs at the end of the previous line. >+ allowPort : function GP_allowPort(port, scheme) { >+ return true; // meaningless. It might be meaningless, but it should still return false. >+ newURI : function GP_newURI(spec, charset, baseURI) { >+ var uri = Components.classes["@mozilla.org/network/standard-url;1"]. >+ createInstance(Components.interfaces.nsIURI); >+ uri.spec = spec; Ideally this would use nsIStandardURL and call init(URLTYPE_STANDARD, this.defaultPort, spec, charset, baseURI) (URL_TYPE_STANDARD is a constant on nsIStandardURL) >+ var chan = Components.classes["@mozilla.org/network/io-service;1"]. >+ getService(Components.interfaces.nsIIOService). >+ newChannel("chrome://communicator/content/gopher/gopherAddon.xhtml", null, null); Nit: . at the start of the line, lined up with the . in the previous line. (Same below.) >diff --git a/suite/installer/removed-files.in b/suite/installer/removed-files.in I don't know how this file works, check with someone else please? >+<!ENTITY loadError.label "Gopher servers require an add-on"> "Page Load Error" >+<!ENTITY goToAddOn.label "Go To Add-on Page"> Nit: This goes last in the file. >+<!ENTITY gopherAddon.title "Add-on Required"> "Gopher Protocol" >+<!ENTITY gopherAddon.shortDesc "Gopher URLs require an add-on to access them."> "gopher is not a registered protocol." >+<!ENTITY gopherAddon.longDesc "You can get a compatible add-on to access this server from Mozilla Add-ons."> Prefix this sentence: "The address specifies the gopher protocol which is no longer supported, so the browser cannot connect to the site."
Attachment #8350866 -
Flags: review?(neil) → review-
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8350866 [details] [diff] [review] proposed patch (v1) requesting review on specifically the removed-files.in part.
Attachment #8350866 -
Flags: review?(bugspam.Callek)
Assignee | ||
Comment 15•10 years ago
|
||
Attachment #8350866 -
Attachment is obsolete: true
Attachment #8350866 -
Flags: review?(bugspam.Callek)
Attachment #8351087 -
Flags: review?(neil)
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8351087 [details] [diff] [review] proposed patch (v2) Specific review for the changes in removed-files.in.
Attachment #8351087 -
Flags: review?(bugspam.Callek)
Comment 17•10 years ago
|
||
Comment on attachment 8351087 [details] [diff] [review] proposed patch (v2) >+ <link rel="stylesheet" href="chrome://global/skin/netError.css" type="text/css" /> Nit: still no space before /> please. >+ document.location = "http://addons.mozilla.org/addon/7685"; Still need to decide whether we're allowed to hard-code URLs like this or whether they should be properties. (So don't bother uploading a new patch until then.) Oh, and this needs to be https: of course. >+ return false // meaningless. Nit: missing semicolon >+<!ENTITY goToAddOn.label "Go To Add-on Page"> Nit: Go to
Comment 18•10 years ago
|
||
Comment on attachment 8351087 [details] [diff] [review] proposed patch (v2) OK, looks like we're good to go with the hard-coded URL, in which case r=me with those nits fixed and the packaging fixes independently checked.
Attachment #8351087 -
Flags: review?(neil) → review+
Assignee | ||
Comment 19•10 years ago
|
||
Attachment #8351087 -
Attachment is obsolete: true
Attachment #8351087 -
Flags: review?(bugspam.Callek)
Attachment #8354987 -
Flags: review+
Assignee | ||
Comment 20•10 years ago
|
||
Comment on attachment 8354987 [details] [diff] [review] proposed patch(v3) Asking review for the packaging part of this patch.
Attachment #8354987 -
Flags: review?(bugspam.Callek)
Comment 21•10 years ago
|
||
Comment on attachment 8354987 [details] [diff] [review] proposed patch(v3) Review request for packaging part
Attachment #8354987 -
Flags: review?(bugzilla)
Comment 22•10 years ago
|
||
Comment on attachment 8354987 [details] [diff] [review] proposed patch(v3) Edmund: In general the packaging changes are ok, but you don't need the change to suite/installer/removed-files.in. That section in removed-files.in is for files that were shipped before we enabled omnijar by default, so that they get removed on update. I know some people added more and more files to that section after we enabled omnijar, but it's not really necessary (and slows down the installer a tiny bit). We only ship official releases with omnijar enabled.
Attachment #8354987 -
Flags: review?(bugzilla) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8354987 -
Flags: review?(bugspam.Callek)
Assignee | ||
Comment 23•10 years ago
|
||
This patch is without the removed-files.in changes.
Attachment #8354987 -
Attachment is obsolete: true
Attachment #8362042 -
Flags: review+
Assignee | ||
Comment 24•10 years ago
|
||
Pushed to comm-central: https://hg.mozilla.org/comm-central/rev/8e381db999ec
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Target Milestone: --- → seamonkey2.28
You need to log in
before you can comment on or make changes to this bug.
Description
•