Closed Bug 1452645 Opened 6 years ago Closed 6 years ago

Remove "Open in Sidebar" feature

Categories

(Firefox :: Bookmarks & History, enhancement, P1)

Product:
Firefox
Component:
Bookmarks & History
Version:
50 Branch
Type:
enhancement

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 63
Tracking Flags:
Tracking Status
relnote-firefox --- 63+
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 + fixed

People

(Reporter: mixedpuppy, Assigned: standard8)

References

Details

(Keywords: dev-doc-complete, devrel-needed, site-compat, Whiteboard: [killthem])

Attachments

(3 files, 1 obsolete file)

bmsidebar.xpi
1.01 KB, application/x-xpinstall
Details
Bug 1452645 - Remove load in sidebar functionality.
59 bytes, text/x-review-board-request
mak
: review+
lina
: review+
Details
Bug 1452645 - Remove now unused PlacesTransactions.Annotate.
59 bytes, text/x-review-board-request
mak
: review+
Details 
Bookmarks have a "open in sidebar" option.  This loads the bookmark into a web panel sidebar.

This functionality has received no love and has lots of issues.  

- It is not remoted.  
- Things like datetimepicker do not work (see bug 1450473 for web extension sidebar).  
- There are a handful of old bugs (e.g. bug 476719, bug 780503).
- Added complexity supporting it.
- The webapi for adding a web sidebar was removed[1]

I thought at one time there was a bug to remove it, but I cannot locate it.

So I'm opening this to get a decision on its status.  Either it needs to be brought up to date, or it should be removed.

If it is to be brought up to date, we should probably refactor all the sidebar code for handling the extension sidebar and the web sidebar into a single browser sidebar.  I originally separated the two due to my understanding that the web sidebar was going away, and because I was uncertain at the time what differences would exist.

[1] https://developer.mozilla.org/en-US/docs/Web/API/Window/sidebar
Flags: needinfo?(dolske)
I guess this is ultimately a product decision.

I don't feel strongly either way, but am gently in favor of removing it, for the reasons you note. It's always been a bit of an oddball feature IMO. The only thing I can think of to the contrary is that sidebars have been mildly more in favor as of late (e.g. our improvements in Photon), likely since they can be a good way to use some realestate on widescreeen monitors.
Flags: needinfo?(dolske) → needinfo?(pdolanjski)
We are about to run an experiment with this feature in Test Pilot. We should get some signal if this feature is worth maintaining.

I'll leave PDol NI'ed for now.
We're consuming this https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/sidebarAction/setPanel, so i'm not exactly sure we'd be overlapping here?
(In reply to [:jgruen] from comment #3)
> We're consuming this
> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/sidebarAction/
> setPanel, so i'm not exactly sure we'd be overlapping here?

That is the webextension sidebar, not the web panel sidebar.  The only way to open a web panel sidebar is to have a bookmark and set the option to "open in sidebar".
(In reply to Shane Caraveo (:mixedpuppy) from comment #4)
> (In reply to [:jgruen] from comment #3)
> > We're consuming this
> > https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/sidebarAction/
> > setPanel, so i'm not exactly sure we'd be overlapping here?
> 
> That is the webextension sidebar, not the web panel sidebar.  The only way
> to open a web panel sidebar is to have a bookmark and set the option to
> "open in sidebar".

IOW, this bug does not have any affect on what you are doing.
Cool, thanks! Worth noting that i'd probably be pretty simple to make webExtension that could pretty easily replace the behavior you want to deprecate. There's a option in the menu API to target context clicks on bookmarks specifically.

https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/menus/ContextType
Sorry for the long delay.  I agree that this option is so buried and there is an alternative, so I don't really have concerns about dropping the functionality, unless there are common use cases I'm not aware of.
Flags: needinfo?(pdolanjski)
I'm actually pretty certain that this functionality can also just be created with a web extension now, using the web extension sidebar.  I'll try to mock up an extension to do it.  If it can, then I think there is a replacement for those who want it.  That would let us drop a bunch of code specific to that feature.
and...now I notice comment 6 :)
Attached file bmsidebar.xpi — 
And there we go, a context menu to open bookmarks in the extension sidebar.
Here's a quick beginning.  There is still a lot in the places (database) and sync components that I am leery of touching.  This at least gets it out of UI and removes a chunk of sidebar code.
Assignee: nobody → mixedpuppy
Any objections to duping to bug 560305? It has a handful of dependencies, so I wouldn't forward-dupe to this.
Component: Untriaged → Bookmarks & History
Flags: needinfo?(mixedpuppy)
Whiteboard: [killthem]
I would prefer duping to this, but I suppose it doesn't really matter.
Flags: needinfo?(mixedpuppy)
Blocks: 673079
I totally support this removal!
Blocks: 1460577
Priority: -- → P2
See Also: → 1461988
Discussed with Shane and he said I could take this over, so taking...
Assignee: mixedpuppy → standard8
Priority: P2 → P1
Attachment #8984508 - Flags: review?(kit)
Possibly things still to figure out:

- Is rel="sidebar" defined / supported somewhere else, such that we need to remove it from core or something?
- A possible intent to unship notice due to the rel="sidebar".
Comment on attachment 8984508 [details]
Bug 1452645 - Remove load in sidebar functionality.

https://reviewboard.mozilla.org/r/250362/#review256744

Yay for removing more cruft! Sync parts look good, thanks.
Attachment #8984508 - Flags: review?(kit) → review+
Attachment #8971690 - Attachment is obsolete: true
(In reply to Mark Banner (:standard8) from comment #21)
> Possibly things still to figure out:
> 
> - Is rel="sidebar" defined / supported somewhere else, such that we need to
> remove it from core or something?

This is now fixed in the latest patch. There was some handling in content.js that needed removing.

> - A possible intent to unship notice due to the rel="sidebar".

Given the soft freeze is basically upon us, I'm intending to send this out next week and land the patch after the soft freeze, i.e. in the 63 cycle.
Comment on attachment 8984509 [details]
Bug 1452645 - Remove now unused PlacesTransactions.Annotate.

https://reviewboard.mozilla.org/r/250364/#review257468
Attachment #8984509 - Flags: review?(mak77) → review+
Comment on attachment 8984508 [details]
Bug 1452645 - Remove load in sidebar functionality.

https://reviewboard.mozilla.org/r/250362/#review257520

There is a call to getPanelBrowser at https://searchfox.org/mozilla-central/source/browser/base/content/browser-context.inc#274

Also, webext-panels.xul calls into PanelBrowserStop() and PanelBrowserReload(); that you are removing
Thus, it looks like webext depend on some of this removed code...

::: browser/base/content/browser.js
(Diff revision 3)
> -
> -function asyncOpenWebPanel(event) {
> -  if (gWebPanelURI && SidebarUI.browser.contentDocument &&
> -      SidebarUI.browser.contentDocument.getElementById("web-panels-browser")) {
> -    SidebarUI.browser.contentWindow.loadWebPanel(gWebPanelURI);
> -    SidebarUI.setWebPageIcon(gWebPanelURI);

Looks like setWebPageIcon can be removed

::: browser/components/places/content/bookmarkProperties.js:451
(Diff revision 3)
> -      annotations.push({ name: PlacesUIUtils.LOAD_IN_SIDEBAR_ANNO,
> -                         value: true });
> -    }
>  
>      let itemGuid;
> -    let info = { parentGuid, index, title: this._title, annotations };
> +    let info = { parentGuid, index, title: this._title };

please file a bug to remove annotations support from PlacesTransactions once we removed all the critical ones.
Attachment #8984508 - Flags: review?(mak77) → review-
Summary: web sidebar, should it stay or should it go? → Remove "Open in Sidebar" feature
Blocks: 1469698
Comment on attachment 8984508 [details]
Bug 1452645 - Remove load in sidebar functionality.

https://reviewboard.mozilla.org/r/250362/#review257520

I did some digging. On the PanelBrowserStop() and PanelBrowserReload() front, it turns out that webext-panels.xul isn't actually loading web-panels.js and so the commands that use them are broken - I'm not even sure if they actually get called. Hence, I think it is fine to remove them, and I've filed bug 1469763 to fix the WebExtension issue.

For the getPanelBrowser in browser-context.inc, webext-panels.js defines a stub implementation of gBrowser, so they'd never hit that. Hence I've removed it from browser-context.inc. I also filed another WebExtension bug here (bug 1469764), as when I was investigating it, it was clear that the context menu isn't working fully even though most of the setup was there.

> please file a bug to remove annotations support from PlacesTransactions once we removed all the critical ones.

Filed bug 1469698.
Comment on attachment 8984508 [details]
Bug 1452645 - Remove load in sidebar functionality.

https://reviewboard.mozilla.org/r/250362/#review258146
Attachment #8984508 - Flags: review?(mak77) → review+
Keywords: site-compat
Setting relnote-firefox? flag for release notes inclusion, and devrel-needed keyword for inclusion in the MDN developer release notes.
relnote-firefox: --- → ?
Keywords: devrel-needed
Blocks: 1467996
Comment on attachment 8984508 [details]
Bug 1452645 - Remove load in sidebar functionality.

https://reviewboard.mozilla.org/r/250362/#review258570

::: toolkit/components/places/SyncedBookmarksMirror.jsm:121
(Diff revision 5)
>  
>  const SQLITE_MAX_VARIABLE_NUMBER = 999;
>  
>  // The current mirror database schema version. Bump for migrations, then add
>  // migration code to `migrateMirrorSchema`.
> -const MIRROR_SCHEMA_VERSION = 2;
> +const MIRROR_SCHEMA_VERSION = 3;

On second thought, would you mind reverting the schema version change, and the change to `mirror.items`? Sorry for the churn, I didn't think this through yesterday. :-/

Like Places, `migrateMirrorSchema` only checks for old schema versions; new schema versions get downgraded without changing the schema itself. So a newer mirror will still fail on older Firefoxes. Then, if you switch back to the newer channel, we'll see the schema version is too old, trash the database, and download everything again.

It's probably better to leave the `loadInSidebar` column in `mirror.items` for now, and remove it in 2-3 cycles, maybe with a comment that it's unused. Or, since we haven't shipped the mirror yet, breaking on profile downgrade might be OK, though the extra column isn't hurting anything.
Attachment #8984508 - Flags: review+
Comment on attachment 8984508 [details]
Bug 1452645 - Remove load in sidebar functionality.

https://reviewboard.mozilla.org/r/250362/#review258570

> On second thought, would you mind reverting the schema version change, and the change to `mirror.items`? Sorry for the churn, I didn't think this through yesterday. :-/
> 
> Like Places, `migrateMirrorSchema` only checks for old schema versions; new schema versions get downgraded without changing the schema itself. So a newer mirror will still fail on older Firefoxes. Then, if you switch back to the newer channel, we'll see the schema version is too old, trash the database, and download everything again.
> 
> It's probably better to leave the `loadInSidebar` column in `mirror.items` for now, and remove it in 2-3 cycles, maybe with a comment that it's unused. Or, since we haven't shipped the mirror yet, breaking on profile downgrade might be OK, though the extra column isn't hurting anything.

No problem. I've reverted those changes, added a comment to mirror.items, and I've kept the removal of loadInSidebar from the itemsToUpload temp table.
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0efc06748a26
Remove load in sidebar functionality. r=lina,mak
https://hg.mozilla.org/integration/autoland/rev/8717824f46dc
Remove now unused PlacesTransactions.Annotate. r=mak
https://hg.mozilla.org/mozilla-central/rev/0efc06748a26
https://hg.mozilla.org/mozilla-central/rev/8717824f46dc
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
The change was added to Firefox Nightly 63 release notes with this wording:
In the Library, the ***Open in Sidebar*** feature for individual bookmarks was removed
status-firefox61: --- → unaffected
status-firefox62: --- → unaffected
status-firefox-esr52: --- → unaffected
status-firefox-esr60: --- → unaffected
tracking-firefox63: --- → +
relnote-firefox: ? → 63+
Blocks: 430959
Blocks: 1369362
Blocks: 1374823
Blocks: 780503
Blocks: 1248927
Blocks: 1248957
Blocks: 1349185
Blocks: 1374799
Blocks: 1426929
If the API support for this feature was removed a long time ago (according to the site compatibility note linked to above) it seems the only need for documentation is a note that the rel="sidebar" attribute of the a tag is now ignored.

I added this to the 63 release notes:

Support for the the sidebar link type (rel = "sidebar") has been removed. If an anchor tag includes this attribute, it will be ignored.

I also edited the link type description of sidebar (https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types) to say:

While once part of the HTML specification, this has been removed from the spec and is only implemented by versions of Firefox prior to Firefox 63.

I don't think this requires a bcd change because before I changed it, this type was described as being supported by Firefox only.

Did I miss something?
Flags: needinfo?(standard8)
Keywords: dev-doc-needed → dev-doc-complete
That all looks good, thank you.
Flags: needinfo?(standard8)
What does it mean when "comment is hidden (advocacy)"?
(In reply to Noam Tamim from comment #49)
> What does it mean when "comment is hidden (advocacy)"?

Noam, this means that you were providing comments to a bug that's already resolved/ closed and that comment contained content that is more appropriate in a forum or mailing list, instead of a bug tracker used for tracking work items.
Being an open source project and organization, we appreciate any and all (volunteer) contributions, but preferably of a constructive kind. You can read more about how we use Bugzilla at https://bugzilla.mozilla.org/page.cgi?id=etiquette.html and comment tagging in particular at https://wiki.mozilla.org/BMO/comment_tagging.
I will add another advocacy comment because this ticket should be reopened and reversed. The sidebar was a perfect spot for my Google Tasks todo list, and I was using it for years with 0 issues because how easy it was to utilize - just click the sidebar button. Now I have to jump through hoops to get to my list, and I often lose my train of thought by the time I get to it, and that's especially irritating knowing how this wasn't a problem before. You need to restore this feature and provide explanation that its functionality is minimal, and complex websites will not work with it. It's as simple as that.
@tweedbeats, we're sorry you're disappointed with this change, unfortunately we can't support every use case and the decision has been made ehre.

Please be aware that there are add-ons available that have alternative functionality that may fit your use cases (and may work even better):

https://addons.mozilla.org/firefox/search/?q=sidebar

I spotted this one in particular that may fit this use case: https://addons.mozilla.org/firefox/addon/webpage-sidebar/
Decisions are made and unmade every day.

The add-on is clunky and unusable for my use case. The solution shouldn't be to add an ugly button with a visibly ugly name on my main toolbar/sidebar. Or I guess I could keep the button in the overflow menu and add another click to get to it? I still have to click that ugly button, and click another time on the site I need opened, then click again elsewhere so the clunky looking list of sites disappears. I have to do this upon every restart of this browser. Firefox actually remembered the last site I had opened in the sidebar, even though it might've not been the specific bookmarked page with the "Load in sidebar" checked.  This functionality is now lost for no good reason.

"unfortunately we can't support every use case "

That was my line. Don't support every use case, since the feature was already providing great value to a lot of people as it was - a clarification that complex sites might not work properly should've been the only change here. And I bet there are other list managers that will work flawlessly in the sidebar, and Mozilla should be advertising this, not needlessly cutting down itself from properly working features lol.


I'll just add that I've been an user since Netscape days as well.
(In reply to Mark Banner (:standard8) from comment #52)
 
> I spotted this one in particular that may fit this use case:
> https://addons.mozilla.org/firefox/addon/webpage-sidebar/

So far the Addon does not fix the problem for us either, we have a custom internal bookmark and navigation server which is build around the sidebar feature. And it does not work with the webpage-sidebar addon since it does not properly open links in the main window as one would expect from a sidebar.

Provided that several hundred people already looked (searched the net) for a solution to fix this "problem", removing this feature clearly created a problem for a number of people and reading thru the discussion here, I am missing the good reason to do it.
If it would have been seriously broken and none could be found to fix it, that would have at least been a reason to remove it, but removing it because basically a dozen people do not see its value does not strike me as a good choice.
(In reply to Mark Banner (:standard8) from comment #52)
> @tweedbeats, we're sorry you're disappointed with this change, unfortunately
> we can't support every use case and the decision has been made ehre.
> 
> Please be aware that there are add-ons available that have alternative
> functionality that may fit your use cases (and may work even better):
> 
> https://addons.mozilla.org/firefox/search/?q=sidebar
> 
> I spotted this one in particular that may fit this use case:
> https://addons.mozilla.org/firefox/addon/webpage-sidebar/

I don't mean to be rude, but I really can't figure out what exactly do you need to support here? I assume you still intend to support opening web pages in the sidebar, right? I haven't looked at the bookmarks code, but I don't see how one option in a bookmark and one function call can be so complicated?

None of the things mentioned :mixedpuppy were really fixed. All things except maybe Sync problem don't apply. And I don't think you can Sync, because sidebar doesn't make sense on mobile.

And also it's not really feasible to do it with an extensions as each extension has one button. So you would have to create an extension for each sidebar bookmark... So that seems like an absurd amount of work compared to work needed to support a simple checkbox.
(In reply to Maciej Jaros from comment #55)

> I don't mean to be rude, but I really can't figure out what exactly do you
> need to support here? I assume you still intend to support opening web pages
> in the sidebar, right? I haven't looked at the bookmarks code, but I don't
> see how one option in a bookmark and one function call can be so complicated?

No, your assumption is wrong. They've completely removed the option to open pages in the sidebar. No more right-click -> open in sidebar, no more "open bookmark in sidebar". All gone.

Like I wrote in comment 48 (hidden for being "advocacy"), Firefox is becoming more and more like Chrome (in the sense of being dumbed-down). As part of that, they are removing power-user features like live bookmarks and this one.

Dear Firefox and Mozilla, I've loved you for 14 years. I convinced friends and family to ditch IE 6 for you. I stayed with you when said F&F moved to Chrome. But you're killing my favorite power features and it's getting harder to love you.

I'm staying, for now. And I'll keep my monthly PayPal donation because I believe there should be a truly free alternative to Edge and Chrome. 

But I don't know for how long.
(In reply to Noam Tamim from comment #56)
> (In reply to Maciej Jaros from comment #55)
> 
> > I don't mean to be rude, but I really can't figure out what exactly do you
> > need to support here? I assume you still intend to support opening web pages
> > in the sidebar, right? I haven't looked at the bookmarks code, but I don't
> > see how one option in a bookmark and one function call can be so complicated?
> 
> No, your assumption is wrong. They've completely removed the option to open
> pages in the sidebar. No more right-click -> open in sidebar, no more "open
> bookmark in sidebar". All gone.

No. The future to open a web page in a sidebar is still there and available for extensions. I checked FF Nightly 65 and Open in Sidebar extension/add-on is still working.
https://addons.mozilla.org/pl/firefox/addon/open-link-in-sidebar/

Although this extensions works it is not feasible to recreate previous experience with a web extension (as I explained before).

I assume from the discussion that the feature to open a web page in a sidebar is meant to stay. I still would be grateful at least for a confirmation that this is the case. That there is intent to support this feature.
^The feature (not "The future") :-). Sorry.
The WebExtension API is considered mostly stable, as such if it supports opening in the sidebar (and it does afaict) it will keep supporting that. There has been experimentation about side by side view as well (https://blog.mozilla.org/firefox/its-a-new-firefox-multi-tasking-extension-side-view/), thus people in charge of evaluating user experience are well informed of needs and downsides.
This bug is only about the bookmarks functionality.
An enterprise that needs a bunch of pages in the sidebar could easily create an add-on for that and install it through policies.

Note that me-too and advocacy comments are not particularly useful on a technical issues tracker, as such please refrain from them, or we may have to close comments to reduce mail traffic.
As I tried to explain before it is NOT POSSIBLE to recreate this function with an extension.

You can easily add bookmarks. Bookmarks are buttons with text and icon that can be placed on a top toolbar. You can access bookmarks with one click.

You cannot make an extension like that. Extensions as of FF Quantum can only have one button per extension. You could create an extension witha a menu (two clicks), but not with a button (one click).


Also this bug report is invalid or at least not resolved. Let's go one-by-one from the opening comment:

 - "It is not remoted." -- not convinced it needs to be synced (explained above)
 - "Things like datetimepicker do not work (see bug 1450473 for web extension sidebar)." -- removing bookmarks option didn't fix the sidebar
 - "There are a handful of old bugs (e.g. bug 476719, bug 780503)." -- both bugs are not really bookmark related. Same problems would occur with a page opened by an extension.
 - "Added complexity supporting it." -- one checkbox is hardly complex...
 - "The webapi for adding a web sidebar was removed[1]" -- not true. webextension API is still there.

So, none of the reported problems were fixed. Simple yet useful feature was removed. No warning was given to users.
(In reply to Maciej Jaros from comment #60)

> Also this bug report is invalid or at least not resolved. Let's go
> one-by-one from the opening comment:
> 

Well, my comment above (about your assumption being wrong) related to what this bug claims to solve (removing the API etc).

In addition, I don't understand something. In the comments preceding my comment (the hidden one that sparkled this post-mortem discussion) all the devs seem really happy that they removed APIs and "cruft". Now you're saying the APIs are still there, and just the checkbox is removed?

The "Load this bookmark in the sidebar" feature is very useful to me.
I can have a useful tool side by side with a site I'm viewing.

My tools:

People should be happy with updating Firefox, but disappearing features makes them angry.

I think Mozilla developers can do better. Before removing a feature, an alternative addon by Mozilla or WebExtension by Mozilla should be developed, no third-party. Then let a user be almost unaware that a feature implementation has changed.

(In reply to dariusz.kierzkowski from comment #62)

The "Load this bookmark in the sidebar" feature is very useful to me.
I can have a useful tool side by side with a site I'm viewing.

My tools:

People should be happy with updating Firefox, but disappearing features makes them angry.

I think Mozilla developers can do better. Before removing a feature, an alternative addon by Mozilla or WebExtension by Mozilla should be developed, no third-party. Then let a user be almost unaware that a feature implementation has changed.

¡Hola Dariusz!

Looks like https://addons.mozilla.org/firefox/addon/side-view/ might help with your use case.

Verified fixed on Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0 ID:20190211215545

¡Gracias!
Alex

Status: RESOLVED → VERIFIED

I'm too a user of the first version of Firefox and I did my donations as well. But yes, it is getting harder to stick with it - and the arguments not to fall into the claws of Chrome&Co. are getting less and less.
And besides the fact that it is extremely rude and unprofessional to 'remove features because we think so' it is the way you are responding to criticism in that regard. Open up your eyes - the web is full of complains in that direction ( like 'I've got the feeling it won't be too long before I give up on Firefox completely.'). If you decide to continue this way - your way - you could save us a long painful way of watching Firefox going down in stopping this project right now.

Oh, and btw, yes - I've used 'Load into sidebar' for years to load my own little hand-crafted page with all my favourite links (Intranet and Internet) organized in a nice and clear tree. None of the advertised plug-ins do work exactly like this. One is refusing to load a local page from file-system the other one does not automatically load on startup ...

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

Attachment

General

Creator:
Created:
Updated:
Size: