Last Comment Bug 468337 - Remove "what's related" sidebar/module
: Remove "what's related" sidebar/module
Status: RESOLVED FIXED
: fixed1.9.1
Product: SeaMonkey
Classification: Client Software
Component: Sidebar (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Robert Kaiser
:
Mentors:
Depends on:
Blocks: 13226 57127 63608 78821 81127 87084 122723 139894 147155 158269 221833 222813 253093 319170
  Show dependency treegraph
 
Reported: 2008-12-07 08:00 PST by Robert Kaiser
Modified: 2008-12-17 17:56 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
What's Related sidebar WFM. (134.23 KB, image/png)
2008-12-07 12:48 PST, Philip Chee
no flags Details
remove "what's related" (43.45 KB, patch)
2008-12-13 16:21 PST, Robert Kaiser
mnyromyr: review+
neil: superreview+
Details | Diff | Splinter Review
supplementary Mozilla patch (3.56 KB, patch)
2008-12-14 15:49 PST, Robert Kaiser
mozilla: review+
neil: superreview+
Details | Diff | Splinter Review
comm-central patch (27.44 KB, patch)
2008-12-14 16:00 PST, Robert Kaiser
mnyromyr: review+
neil: superreview+
Details | Diff | Splinter Review

Description Robert Kaiser 2008-12-07 08:00:28 PST
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.
Comment 1 Philip Chee 2008-12-07 12:48:48 PST
Created attachment 351811 [details]
What's Related sidebar WFM.

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?
Comment 2 Karsten Düsterloh 2008-12-07 13:15:49 PST
> Perhaps this is a Linux/OSX specific problem?

No problem under Linux, it's WFM for both trunk and branch...
Comment 3 Stefan [:stefanh] 2008-12-07 14:43:54 PST
I'm not against removing it, but it's wfm on OS X.
Comment 4 Tony Mechelynck [:tonymec] 2008-12-07 15:22:03 PST
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.
Comment 5 Philip Chee 2008-12-07 20:11:36 PST
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>
Comment 6 Robert Kaiser 2008-12-13 16:21:02 PST
Created attachment 352867 [details] [diff] [review]
remove "what's related"

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.
Comment 7 Philip Chee 2008-12-13 16:44:00 PST
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 8 neil@parkwaycc.co.uk 2008-12-14 04:25:32 PST
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 ?
Comment 9 Robert Kaiser 2008-12-14 05:33:03 PST
(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?
Comment 10 Robert Kaiser 2008-12-14 06:03:35 PST
Comment on attachment 352867 [details] [diff] [review]
remove "what's related"

Handing an additional review to Mnyromyr, who is the SeaMonkey Sidebar owner.
Comment 11 Robert Kaiser 2008-12-14 07:48:08 PST
(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 12 neil@parkwaycc.co.uk 2008-12-14 09:18:07 PST
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.
Comment 13 Karsten Düsterloh 2008-12-14 13:25:26 PST
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.
Comment 14 Peter Weilbacher 2008-12-14 14:55:00 PST
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.
Comment 15 Robert Kaiser 2008-12-14 15:49:49 PST
Created attachment 352943 [details] [diff] [review]
supplementary Mozilla patch

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.
Comment 16 Robert Kaiser 2008-12-14 16:00:46 PST
Created attachment 352948 [details] [diff] [review]
comm-central patch

Here's the comm-central parts - with those that sidebar should really be gone for good.
Comment 17 Robert Kaiser 2008-12-15 11:10:41 PST
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.
Comment 18 neil@parkwaycc.co.uk 2008-12-17 04:22:26 PST
Comment on attachment 352948 [details] [diff] [review]
comm-central patch

I couldn't think of anywhere else where that image would be useful.
Comment 19 Robert Kaiser 2008-12-17 10:10:42 PST
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.
Comment 20 Serge Gautherie (:sgautherie) 2008-12-17 17:26:18 PST
(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.
Comment 21 Robert Kaiser 2008-12-17 17:56:34 PST
(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.

Note You need to log in before you can comment on or make changes to this bug.