Remove "what's related" sidebar/module

RESOLVED FIXED

Status

SeaMonkey
Sidebar
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Robert Kaiser, Assigned: Robert Kaiser)

Tracking

({fixed1.9.1})

Trunk
fixed1.9.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Assignee)

Description

9 years ago
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

9 years ago
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

9 years ago
> Perhaps this is a Linux/OSX specific problem?

No problem under Linux, it's WFM for both trunk and branch...

Comment 3

9 years ago
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.

Comment 5

9 years ago
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>
(Assignee)

Comment 6

9 years ago
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.
Assignee: nobody → kairo
Status: NEW → ASSIGNED
Attachment #352867 - Flags: superreview?(neil)
Attachment #352867 - Flags: review?(neil)

Comment 7

9 years ago
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

9 years ago
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-
(Assignee)

Comment 9

9 years ago
(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?
(Assignee)

Updated

9 years ago
Attachment #352867 - Flags: review?(mnyromyr)
(Assignee)

Comment 10

9 years ago
Comment on attachment 352867 [details] [diff] [review]
remove "what's related"

Handing an additional review to Mnyromyr, who is the SeaMonkey Sidebar owner.
(Assignee)

Comment 11

9 years ago
(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 13

9 years ago
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+

Comment 14

9 years ago
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.
(Assignee)

Comment 15

9 years ago
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.
Attachment #352943 - Flags: superreview?(neil)
Attachment #352943 - Flags: review?(mozilla)
(Assignee)

Comment 16

9 years ago
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.
Attachment #352948 - Flags: superreview?(neil)
Attachment #352948 - Flags: review?(mnyromyr)

Updated

9 years ago
Attachment #352943 - Flags: superreview?(neil) → superreview+

Updated

9 years ago
Attachment #352943 - Flags: review?(mozilla) → review+
(Assignee)

Comment 17

9 years ago
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

Updated

9 years ago
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+
(Assignee)

Comment 19

9 years ago
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
Last Resolved: 9 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.
(Assignee)

Comment 21

9 years ago
(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.