Closed Bug 613974 Opened 11 years ago Closed 8 years ago

"Load this bookmark in the sidebar" checkbox for bookmarks is not available/working in SeaMonkey [browserWin.openWebPanel is not a function]

Categories

(SeaMonkey :: Sidebar, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.30

People

(Reporter: Manuel.Spam, Assigned: InvisibleSmiley)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

Part of the places bookmark system, as ported from Firefox, is a checkbox which allowes to open a bookmark into sidebar.

IMHO it's a pretty bad idea to even try to implement this into the current sidebar. The current way of adding sidebars from websites (store them into panels.rdf) should be dropped at all and all sidebars, added from websites, should be bookmarks (just as Firefox does). To be able to do this, sidebar has to be replaced with new code (maybe xsidebar) first.
Depends on: 613971
So, to be clear, bug 399310 hasn't enabled this to work? Why not? Isn't this way of opening a web page as a sidebar also part of the "Firefox Sidebar API"?
There are other parts of the API that need to be ported. c.f. xSidebar.
Blocks: 113994
Bug 339310 was about adding the "broadcaster based" way of adding sidebars.

Addons add an overlay to the browser XUL and an entry for their sidebar to the "mainBroadcasterSet". In Firefox this causes the sidebar to be available via "View -> Sidebar". On SeaMonkey, it now causes the sidebar to be available via the current "Sidebar customize" stuff (RDF).

See also: https://developer.mozilla.org/en/Creating_a_Firefox_sidebar#overlay_xul

Places bookmarks are a totally difficult thing and are nearly impossible to add with the current sidebar code whithout causing even more confusion as already caused by the current sidebar design.
Blocks: FF2SM
Duplicate of this bug: 695425
Summary: "open bookmark in sidebar" checkbox for bookmarks should be available/working in SeaMonkey → "Load this bookmark in the sidebar" checkbox for bookmarks should be available/working in SeaMonkey [browserWin.openWebPanel is not a function]
My vote is to remove this functionality altogether from Seamonkey as it doesn't work so nobody is using it, and it doesn't even seem like a good idea anyway.  Any objections?
(In reply to Jeremy Morton from comment #5)
> My vote is to remove this functionality altogether from Seamonkey as it
> doesn't work so nobody is using it, and it doesn't even seem like a good
> idea anyway.  Any objections?

I support this move. It never worked, so it should not have been there in the first place. If anyone wants to have this functionality, they should implement it - but that can easily be done after the removal, too. Changing summary to allow this bug to go either way.

Jeremy, do you want to take this? Feel free to go ahead.
Summary: "Load this bookmark in the sidebar" checkbox for bookmarks should be available/working in SeaMonkey [browserWin.openWebPanel is not a function] → "Load this bookmark in the sidebar" checkbox for bookmarks is not available/working in SeaMonkey [browserWin.openWebPanel is not a function]
Version: unspecified → Trunk
Attached patch bug613974.patchSplinter Review
Not taking yet, but I think it's really time to get rid of the broken UI element that we've been dragging along for so long. The attached patch also cleans up a bit behind the scenes (tried not to touch APIs) but if it's preferred to only remove the XUL element and/or l10n, fine with me. Just please let's not leave it as is any longer, it's really horrible UX.

This is what I used to check for occurrences:
.../suite # grep -riE "loadInSidebar|load_in_sidebar|loadbookmarkinsidebar" *
Attachment #8442993 - Flags: review?(neil)
Comment on attachment 8442993 [details] [diff] [review]
bug613974.patch

Seems reasonable from code inspection.
Attachment #8442993 - Flags: review?(neil) → review+
I've lately run into issues building SM on both Windows and Linux so I cannot verify myself whether my patch is safe. Philip (or anyone else reading this), can you apply the patch on this bug and check for obvious regressions? It should only remove the "Load this bookmark in the sidebar" checkbox; anything else (BM, Bookmarks Toolbar etc.) should still work as in a build without the patch.
Flags: needinfo?(philip.chee)
Comment on attachment 8442993 [details] [diff] [review]
bug613974.patch

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #9)
> I've lately run into issues building SM on both Windows and Linux so I
> cannot verify myself whether my patch is safe. Philip (or anyone else
> reading this), can you apply the patch on this bug and check for obvious
> regressions? It should only remove the "Load this bookmark in the sidebar"
> checkbox; anything else (BM, Bookmarks Toolbar etc.) should still work as in
> a build without the patch.

I took this for a spin and I didn't spot any obvious regressions. f+
Attachment #8442993 - Flags: feedback+
Flags: needinfo?(philip.chee)
Assignee: nobody → jh
Status: NEW → ASSIGNED
Pushed to comm-central:
https://hg.mozilla.org/comm-central/rev/f6c40188e2d4
Target Milestone: --- → seamonkey2.30
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
No longer blocks: FF2SM
You need to log in before you can comment on or make changes to this bug.