Closed Bug 468337 Opened 15 years ago Closed 15 years ago

Remove "what's related" sidebar/module

Categories

(SeaMonkey :: Sidebar, defect)

defect
Not set
normal

Tracking

(seamonkey2.53fixed fixed, seamonkey2.57esrfixed fixed)

RESOLVED FIXED
Tracking Status
seamonkey2.53 fixed fixed
seamonkey2.57esr fixed fixed

People

(Reporter: kairo, Assigned: kairo)

References

Details

(Keywords: fixed1.9.1)

Attachments

(4 files)

The "What's related" sidebar has been empty for a long time and we didn't really hear complaints, so we better just remove that feature.

This is one more old unmaintained piece of xpfe to kill :)

When I looked if there's a bug already filed on that, I did a Bugzilla fast search for the term "related", and subsequently added all the "What's Related" bugs I found as depending on this, so that we have them all linked here and can resolve them when this is being done.
There are other reasons for removing the What's Related sidebar but "not working" isn't one of them. I am on WinXP and this sidebar is still working and as far as I know it never stopped. Perhaps this is a Linux/OSX specific problem?
> Perhaps this is a Linux/OSX specific problem?

No problem under Linux, it's WFM for both trunk and branch...
I'm not against removing it, but it's wfm on OS X.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20081207 SeaMonkey/2.0a3pre - Build ID: 20081207072048

Normally I don't use it, but I just tried it. It works, but not without problems: I have to "Reload" it twice to see the new contents after changing tabs: the first reload sets the sidebar to a grey BG with the text "This function is not available at the moment" (or something similar); the second reload shows the W'R for the new tab.
The Related sidebar code doesn't understand tabs. We could port the DOMi sidebar listener over.

However I should point out that there have been privacy concerns about the Alexa sidebar that sends individually identifiable information in the GET (or is it post) URL sent to the Alexa server. This might be enough to have it removed. Also see:
<http://xsidebar.mozdev.org/modified.html#alexa>
<https://addons.mozilla.org/en-US/firefox/addon/848>
I still think we should remove this sidebar, for multiple reasons: This is probably not used a lot nowadays, it's perfect extension fodder, there is an extension out there, it has privacy implications as told here in comments, there is no need to do binary components for this, and it's code nobody in the SeaMonkey team actively maintains.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #352867 - Flags: superreview?(neil)
Attachment #352867 - Flags: review?(neil)
The C++ code appears to be there only to support the Netscape "Related Links" button. I don't think that the Mozilla Suite ever had this.
Comment on attachment 352867 [details] [diff] [review]
remove "what's related"

Looks like you've got some downloadmanager patches applied ;-)

What about moz_IDs_Input.lst ?
Attachment #352867 - Flags: superreview?(neil)
Attachment #352867 - Flags: superreview+
Attachment #352867 - Flags: review?(neil)
Attachment #352867 - Flags: review-
(In reply to comment #8)
> (From update of attachment 352867 [details] [diff] [review])
> Looks like you've got some downloadmanager patches applied ;-)

No, why do you think that? Because I removed everything xpfe download-manager _doesn't_ need?
This is all just removal of related and some related (no pun intended) cleanup. I realized the only suite-specific thing we have left in xpfe/components after this is the download manager, and it has its own ifdef flag, so it doesn't need MOZ_SUITE there any more and we can move away from using it.

> What about moz_IDs_Input.lst ?

What's that? Where is it? What should be done with it?
Attachment #352867 - Flags: review?(mnyromyr)
Comment on attachment 352867 [details] [diff] [review]
remove "what's related"

Handing an additional review to Mnyromyr, who is the SeaMonkey Sidebar owner.
(In reply to comment #8)
> What about moz_IDs_Input.lst ?

Ah, http://mxr.mozilla.org/comm-central/source/mozilla/modules/plugin/os2wrapper/moz_IDs_Input.lst#727 is what you mean... We might need to ask the OS/2 people about this...
Comment on attachment 352867 [details] [diff] [review]
remove "what's related"

KaiRo explained to me that the REQUIRES line was suite-only stuff and that only the download manager is left.
Attachment #352867 - Flags: review-
Comment on attachment 352867 [details] [diff] [review]
remove "what's related"

> This is probably not used a lot nowadays,

I don't think there's any dependable data to support (or deny) this claim. But I don't think it's wrong, either - and I doubt that it ever has been used much, except by Netscape.

> it's perfect extension fodder,

True, but no argument.

> there is an extension out there,

True, but no argument.

> it has privacy implications as told here in comments,

That's the "most valid" reason to remove it, in concordance with the first point above.

> there is no need to do binary components for this,

The binary component "related" you are removing with this patch has *no* connection to the sidebar panel "What's related". Even with this patch applied, the Sidebar panel will still work!

You need a second patch to remove the actual Sidebar panel, see <http://mxr.mozilla.org/comm-central/search?string=related-panel.xul&find=&filter=> and <http://mxr.mozilla.org/comm-central/source/suite/common/related/>.

> and it's code nobody in the SeaMonkey team actively maintains.

It's hard to maintain unused code anyway. ;-)

r=me with moz_IDs_Input.lst#727 removed (see <http://mxr.mozilla.org/comm-central/search?string=moz_IDs_Input.lst&find=&filter=>) and respective okay from the OS/2 people on that.
Attachment #352867 - Flags: review?(mnyromyr) → review+
Yes, if you remove an interface then it should also be removed from moz_IDs_Input.lst. As that file is only used to create moz_IDs_Generated.h (by running moz_IDs.cmd > moz_IDs_Generated.h with the help of a REXX interpreter) you should also remove the 9 lines about "RELATEDLINKSHANDLER" from moz_IDs_Generated.h, too.
I knew there must be more ;-)

OK, here's a supplementary patch, I did a little MXR search and found one more Mozilla place, defining a CID that's not used anywhere in our code.

Peter, please review the OS/2 part - Neil, please (r+)sr that CID removal.

...and now, I also found the comm-central parts. Patch upcoming.
Attachment #352943 - Flags: superreview?(neil)
Attachment #352943 - Flags: review?(mozilla)
Here's the comm-central parts - with those that sidebar should really be gone for good.
Attachment #352948 - Flags: superreview?(neil)
Attachment #352948 - Flags: review?(mnyromyr)
Attachment #352943 - Flags: superreview?(neil) → superreview+
Attachment #352943 - Flags: review?(mozilla) → review+
Pushed attachment 352867 [details] [diff] [review] to trunk as http://hg.mozilla.org/mozilla-central/rev/c584ffe6ef99 and attachment 352943 [details] [diff] [review] as http://hg.mozilla.org/mozilla-central/rev/8b055659db38 and after baking on FF build cycle, pushed attachment 352867 [details] [diff] [review] to branch as http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1173d8ffe346 and attachment 352943 [details] [diff] [review] as http://hg.mozilla.org/releases/mozilla-1.9.1/rev/45be184dfbe8

This is fixed1.9.1 as all Mozilla bits have been pushed, but it's not fixed completely on the trunks, as the comm-central bits are still missing.
Keywords: fixed1.9.1
Attachment #352948 - Flags: review?(mnyromyr) → review+
Comment on attachment 352948 [details] [diff] [review]
comm-central patch

I couldn't think of anywhere else where that image would be useful.
Attachment #352948 - Flags: superreview?(neil) → superreview+
Pushed attachment 352948 [details] [diff] [review] as http://hg.mozilla.org/comm-central/rev/f06738be68e5 and a packages fixup as http://hg.mozilla.org/comm-central/rev/4ea06eb41a9c - this bug should be FIXED now.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
(In reply to comment #19)
> and a packages fixup as http://hg.mozilla.org/comm-central/rev/4ea06eb41a9c -

Shouldn't components/related.xpt be added to suite/installer/removed-files.in too ? (I'm not sure.)
NB: I don't have this file in recent v2.0a_, but I have it in v1.0.9/1.1.14/1.5a.
(In reply to comment #20)
> (In reply to comment #19)
> > and a packages fixup as http://hg.mozilla.org/comm-central/rev/4ea06eb41a9c -
> 
> Shouldn't components/related.xpt be added to suite/installer/removed-files.in
> too ? (I'm not sure.)

No, during the packaging process, it gets linked into browser.xpt, we never ship those smaller .xpt files so we don't need them in removed-files.
You need to log in before you can comment on or make changes to this bug.