Closed Bug 1248268 Opened 4 years ago Closed 3 years ago

Unable to disable "Recently bookmarked"

Categories

(Firefox :: Bookmarks & History, defect, P3, major)

48 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 49
Tracking Status
firefox46 --- unaffected
firefox47 --- disabled
firefox48 --- disabled
firefox49 --- verified

People

(Reporter: marcino245, Assigned: dao)

References

Details

(Keywords: regression, ux-control)

Attachments

(11 files, 3 obsolete files)

64.37 KB, image/png
Details
32.38 KB, image/png
Details
84.58 KB, image/png
Details
49.23 KB, image/png
Details
27.20 KB, image/png
Details
44.00 KB, image/png
Details
136.83 KB, image/jpeg
Details
46.12 KB, image/png
Details
35.59 KB, image/png
Details
40.73 KB, image/jpeg
Details
13.41 KB, patch
mak
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160214030236

Steps to reproduce:

I did not do anything particular, my Nightly 47.0a1 just self-updated just right now and I just accepted to restart the browser to apply the updates. 


Actual results:

After the browser restarted, I noticed that "Recently bookmarked" thing with several recently bookmarked bookmarks appeared in my bookmarks, occupying the top slots and wasting space. 


Expected results:

The way to disable this thing as I don't need this.

I use Firefox few years already and never in my life have seen this thing before, until now, but the posts on Google shows it appeared in 2008 already, so I guess there has been already a way to disable it before and I had it disabled all the time until the newest Nightly appeared and somehow reactivated it (without my permission). But how to disable it again ?

I tried to disable it, but it seems that you can not turn this off, as I searched Google and most of the solutions were to delete it by clicking on it and selecting "delete" or by going to Library by clicking on "Show all bookmarks" or Ctrl + Shift + B and deleting "Recently bookmarked" folder but this does not work in the both cases - when I click on it, there is no "delete" option as there is when clicking on regular bookmarks, and in the second case there is no "Recently bookmarked" folder in Library. 

There are the links I read after typing "firefox delete recently bookmarked" in Google:
https://support.mozilla.org/en-US/questions/1050380
https://support.mozilla.org/pl/questions/1087781
https://support.mozilla.org/en-US/questions/892162
http://forums.mozillazine.org/viewtopic.php?f=38&t=689845

I also tried on a default profile without add-ons (except Ad-Block), but no success.
OS: Unspecified → Windows XP
Hardware: Unspecified → x86
Component: Untriaged → Bookmarks & History
Blocks: 1219804
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: ux-control
OS: Windows XP → All
Hardware: x86 → All
Sounds like it would be easy to do this through a simple userStyle.css hack?
(In reply to Marco Bonardo [::mak] from comment #5)
> Sounds like it would be easy to do this through a simple userStyle.css hack?

The future of userContent.css/userChrome.css is not clear:
https://bugzilla.mozilla.org/show_bug.cgi?id=1046166#c23

I would really like to see a preference for this because bookmarks are a very important part of the browser, probably most of the users use bookmarks and this enhancement can be very annoying, especially for users who know how bookmarks in Firefox work… So it should be ease to disable this.
Attached image Screenshot
I deleted 'Recently Bookmarked' and 'Recent Tags' folders from Bookmarks sidebar,
but 'Recently Bookmarked' menu is still there.
That would be pointless.
(In reply to Sören Hentzschel from comment #6)
> The future of userContent.css/userChrome.css is not clear:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1046166#c23

But it's clear there will be an alternative, either be a new add-on or Stylish. It will still be feasible.
My personal support experience as admin of the German speaking Firefox forum is that not only a few users don't unterstand why they need so much more add-ons compared with Firefox a few months or years ago. They don't want to install a dozen add-ons, especially because a lot of these users already have compat issues all few major releases of Firefox. And with e10s, XUL deprecation and other decisions the add-on situation won't be better in the next months. But that's a different topic.

I perfectly understand that there can't be a preference for everything and add-ons can be a smart solution. It's also my opinion. But this "enhancement" is special because:

a) I think bookmarks are one of the most important features of every browser, so it's really important to make the bookmark experience better not worse. This enhancement ca be an improvement for some users, I agree, but it also can be a decline in usability.
b) This enhancement is highly visible, not a hidden feature, it will affect a lot of users. I think that's a reason to give the user control about this feature. It's okay to enable this per default, but a option to disable it would be very much appreciated. 
c) More mouse movement is needed to access your bookmarks. If you heavily use bookmarks it really makes a difference.
d) My bookmarks menu now needs to be scrolled (!) to see the complete menu. And that with only four bookmarks folders + the 2 default dynamic bookmarks of Firefox. That's not that much.
e) Because of d) the "Show all bookmarks" button is cut off. Technically it's probably no bug. But it looks like a style bug (on Apple OS X).
f) This enhancement is only useful if you want to access your last added bookmarks. I can't believe that this is the main usage of bookmarks. Maybe I am wrong, I don't have numbers, but I don't know anyone who bookmarks a page just to open the same page shortly after. In most cases there were already other bookmarks added and then this enhancement is useless.
g) It's really annoying if you organize your bookmarks in folders and now have clutter again in the bookmarks menu.
h) I can't sync user styles, a preference can be synced across devices or Firefox profiles on the same device.
Attachment #8719300 - Attachment description: Firefox Nightly 47_0a1 Recently Bookmarked 3 (no folder).png → Firefox Nightly 47_0a1 Recently Bookmarked 3 (no Recently Bookmarked in Library).png
My suggestion is:

1) to make Recently Bookmarked a folder, just like Unsorted Bookmarks - this way it will occupy just a single slot in Bookmarks (just like Unsorted Bookmarks) instead of X top slots

2) to make a switch to hide both Recently Bookmarked and Unsorted Bookmarks folders from Bookmarks, as they still occupy 2 slots each (at the moment, when clicking by right mouse button at Unsorted Bookmarks folder and trying to delete it, the "delete" option is greyed out (disabled) ) so at the moment it seems that the problem escalated to being unable to get rid of both Recently Bookmarked and Unsorted Bokmarks slot occupiers.
I'd like some UX input on this. Are the reasons for disabling this legitimate and common enough that we should have some kind of disabling UI? Or should we have a hidden pref? Or just leave the userChrome.css option?
Flags: needinfo?(mverdi)
Sorry, I withdraw from 2nd, I hastened a little, Unsorted Bookmarks seems greatly usefull, I don't need to hide it.

As for 1st, yes that was the point to make Recently Bookmakred be a folder which might regenerate with recently bookmarked bookmarks just like Unsorted Bookmarks regenerate with unsorted bookmarks. Seems reasonable. (Of course if doable).  As for whether removable or hideable, whichever.
userChrome.css works with e10s.
(In reply to Dão Gottwald [:dao] from comment #17)
> userChrome.css works with e10s.

works for some & you guys are considering its removal 

https://bugzilla.mozilla.org/show_bug.cgi?id=1046166#c23

also

http://forums.mozillazine.org/viewtopic.php?f=23&t=2989859&sid=1ecf8d876033e4ad83f31ce8c75913ab

"I have 2 reasons why I need to remove it.
1. I have long bookmark all the links bookmark are properly organized and sorted. the "recently bookmarked" list cause it over flow.
2. I don't want the next person who will use my laptop can see where i surfing "
My suggestion:

An integer pref in range [0-10]. 
0 means the "Recently bookmarked" list is off. 
1-10 is a number of bookmarks, in fact it will be substituted to const kMaxResults from patch in bug 1219804.
(In reply to ElevenReds from comment #18)
> 2. I don't want the next person who will use my laptop can see where i
> surfing "

This is not critical, we never considered bookmarks a privacy leaking information, a person accessing your laptop can figure out far more details than what is visible from a couple bookmarks. The awesomebar will show the same bookmarks, the sidebar and library are at hand. For sensitive information it would be better to not use bookmarks at all. I'm sure there are add-ons covering your need better, for example in 10 seconds I just found https://addons.mozilla.org/it/firefox/addon/hush-private-bookmarking and didn't even try hard.

That said, the problem is clear enough, as well as the possible solutions, let's avoid transforming this bug tracker into a discussion forum.
(In reply to ElevenReds from comment #18)
> (In reply to Dão Gottwald [:dao] from comment #17)
> > userChrome.css works with e10s.
> 
> works for some & you guys are considering its removal 
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1046166#c23

userContent.css and userChrome.css are different things. Please stop this off-topic discussion, as it won't help move this bug forward.
I think we need at least a hidden pref here.

The width of Bookmarks menu is variable. If you bookmark a Web page which title is long enough, the width of the menu widens.

I usually make bookmark folders and care about the length of the folder's name. In other words, my Bookmarks menu is always set within a certain width. When I bookmark a page, it's in a bookmark folder. So I don't have to change name of a bookmark to maintain the length of Bookmarks menu.

But "Recently Bookmarked" has changed the situation. It automatically inserts five bookmarks on the menu. If the bookmarks contain a long name, the width of the menu widens. It's annoying. Do I have to change name of a bookmark every time I bookmark a page? It's burdensome.

For the above reason, I think we need at least a hidden pref to disable the UI. That's my two cents.
Recent bookmarks used to be a folder that could be deleted from the bookmark menu.  It behaved exactly the same way as the unsorted bookmarks folder.  Users understand that bookmarks can be deleted, so I think having the ability to delete restored would be preferable to a preference.
[Tracking Requested - why for this release]:
Somewhere in Tools:Options, there should be a section with various checkbox controls on bookmark behaviors.
Attached image bookmarkissue.jpg
Why do we need 'Recently Bookmarked' to show in top of the Bookmark Drop-down when there is a folder a few lines down that show the same 'Recently bookmarked', but shows up to 10 (I think) ?  

The two are redundant and the top list in the Bookmarks Drop-menu should just go away...it taking up valuable space for those that have a lot of bookmarks.

An 'option' to not display the List at the top of drop-menu should be added or make it 'automatic display' IF.. the user had perhaps removed the 'Recently Bookmarked Folder' perhaps...  

We don't need Recently Bookmarked in 2 places.
The recently bookmarked "folder" will go away, so there won't be any duplication.
Btw, what's the point on repeating the same things over and over? I ensure you we can read text too, not just code :)
I agree with people wanting to have a pref for disabling this "feature"
=> imho bookmarks are a major part of the browser and having 5 entries at the top is either a waste of space or not enough.

I fail to see how this could be useful :
- if someone is really using bookmarks it's a loss of efficiency and space
- if someone is not really using bookmarks having only five most recent bookmarks is too little because as soon he bookmarked his sixth he won't be able to find the first one => these people need a shortcut to unsorted bookmarks not to five most recents. Besides if they succeed finding their older bookmarks does it not prove that it was useless to show only five most recents because they were smart enough to find where bookmarks go?
Can't the recent bookmarks be moved to the bottom of the menu? This way, people with existing bookmarks wont be forced to scroll down to access their bookmarks, while new users who don't have bookmarks can still access the recent bookmarks.
Flags: needinfo?(mverdi)
Flags: a11y-review?
Flags: needinfo?(mverdi) → needinfo?(felipc)
Flags: needinfo?(felipc)
Flags: a11y-review?
Dao or Michael,

Feel free to renominate for tracking but at this time there is not a clear case for tracking
absent more information.
(In reply to Dão Gottwald [:dao] from comment #13)
> I'd like some UX input on this. Are the reasons for disabling this
> legitimate and common enough that we should have some kind of disabling UI?
> Or should we have a hidden pref? Or just leave the userChrome.css option?

We can fix this using the context menu, which by the way, seems incorrect for this section of the menu. 

If we show the same context menu on the "Recently Bookmarked" header as we do on the "Recently Bookmarked" smart folder we can then swap the delete item with a "Hide Recently Bookmarked" item. This could activate an about:config pref for this section of the menu.

If you later wanted to show this section, right-clicking non-active parts of the menu or items like "Show All Bookmarks" could include a "Show Recently Bookmarked" item that would restore it.
Flags: needinfo?(mverdi)
UX comment: I can remove Recently Bookmarked from the bookmarks side bar, or move it to the bottom where I don't see it. But, whatever I do, it is still right on top of the bookmarks menu, with the full list of so-called bookmarks (which has many entries that I never intended to bookmark). So when I do alt-B to get the bookmark menu and type what for years has been the first letter of the bookmark I want (the one listed first with letter in MY list), I get some random thing from the "recently bookmarked" list instead.

So I want to be able to make the whole thing to go away. But if I can even move it to the bottom of the bookmarks MENU, that would be a great improvement. I cannot find where the controls for menu are. They are not in about:config, so far as I can tell.
(In reply to Jonathan Baron from comment #40)
> So I want to be able to make the whole thing to go away.

+1 also the width of bookmarks menu is way wider(compared to the classy compact in FF45)

>I cannot find where the controls for menu are. They are not in
> about:config, so far as I can tell.

Hope the provide it soon Recently bookmarked is a nuisance tbh.
[Tracking Requested - why for this release]:

can this be fixed before uplift to FF47?

Release tracking is required?
I prefer recent folder (new users too would prefer it as it is well looking & does not overcrowd)what the use if i have recent folder set up?

A pref to disable the top would be optimal/for new users a tip showing would help about recent folders and/if the want this implementation or disable this.

Plus it looks ugly.
[Tracking Requested - why for this release]:Will this issue be fixed in Nightly 48.0 life cycle? I can't stand such a long bookmark menu.
Might need a fix to bug 1248267 first, to get the expected context menu type, so we can implement the UX in comment 38.
Priority: -- → P3
Hmm, I don't know if this is important but a month ago I switched from the affected PC (Windows XP Pro SP3 32bit) to a new PC (Windows 7 SP1 64bit Ultimate), and there is no "Recently Bookmarked" thing on the new pc on Firefox (Release). It seems that Windows 7 SP1 64bit Ultimate + Firefox Release (44-45.02) are not affected? Because I still had and have "Recently Bookmarked" thing on my two Win XP + Firefox Release (44-45.02) laptops.
Assignee: nobody → dao+bmo
Attached patch patch (obsolete) — Splinter Review
The way I extended the places context menu is pretty cumbersome. Let me know if you know a better way.
Attachment #8744306 - Flags: review?(mak77)
Hm, I don't think we should extend the places context menu, it's complex and fragile. I think we should remove it from "static" items like these.
So I think we should implement bug 429469. At that point, these menuitems should have no context menu, and we could implement their own contextual menu separately.

Properly having a places menu here would rather require to be able to include 2 places views in this menu, and we can't. Extending the existing menu may create a lot of technical debt for future.

Moreover the current contextual menu on "static" nodes is extremely unclear, since it refers to the "parent" node, and if this project aims at simplifying bookmarking interaction, sounds like a good idea to remove it.
What do you think?
Comment on attachment 8744306 [details] [diff] [review]
patch

Review of attachment 8744306 [details] [diff] [review]:
-----------------------------------------------------------------

I'd probably keep this as a last resort.
Imo the solution of a separate contextual menu will create far less headaches in future, at cost of o a little bit of code duplication.
Attachment #8744306 - Flags: review?(mak77)
What kind of headaches? Are there further plans regarding the context menu I'm unaware of? My patch isn't pretty but the result shouldn't be that hard to handle. I'm not sure that a separate context menu is less cumbersome to implement and to maintain, so I don't think I want to dive into that without a clear value proposition.
Flags: needinfo?(mak77)
Oops, I hadn't seen comment 48 before commenting. Still, I'm not sure about those future headaches.
(In reply to Dão Gottwald [:dao] from comment #50)
> What kind of headaches?

The places context menu code is complex and relies on specific details, like nodes have a _placesNode and being built in a certain way, every time we touch this context menu, imo, the risk to cause a regression on places nodes or on the "static" nodes would be high. Also just the risk to show an option on one that is only valid for the other one...
I'd clearly be happy if both of these "views" would be managed by the same underlying code, but we're not there, and making things more complex doesn't seem to move us any closer to that.

Plus, as I said in comment 48, the current context menu behavior is VERY confusing for users. We are now adding options on top of an already confusing context menu.
Flags: needinfo?(mak77)
My code is pretty self-contained except for the position and selectiontype attributes. I really don't think it makes things significantly more complicated. I'd suggest we take that and evaluate the greater context menu story separately. It sounds like a bigger project and I'd rather not open that can of worms just for this bug.
it's not just this bug there's bug 1248267 that needs a direction, and the existing code would have to be special cased in most points to make it properly support either a good places node, or a made-up fake node.
These nodes have a well specified behavior, that could be hardcoded easily (we don't even need to update commands!), they don't need all the complication of the places context menu code (that has to check if the parent is read-only, if it's a folder, a bookmark, a separator)... Probably some code and strings could be reused, for common ops like open In New Tab and so on...
Additionally the underlying problem in bug 429469 would get a bit harder to fix, cause after this change some static nodes may still want the places context menu, while other won't.
And moreover this seems to adds the show/hide contextual menuitem to every single bookmark, even if it doesn't make any sense to have this option in a submenu, let alone the toolbar (since IIRC we reuse this context menu everywhere?)
(In reply to Marco Bonardo [::mak] from comment #54)
> it's not just this bug there's bug 1248267 that needs a direction, and the
> existing code would have to be special cased in most points to make it
> properly support either a good places node, or a made-up fake node.

That seems orthogonal to this bug. Whether it makes more sense to special-case stuff or implement a separate context menu there, I honestly don't know (which is part of the reason why I don't want to jump ahead here). If we decide to implement a separate context menu for recent bookmarks, I guess we can use the hideRecentlyBookmarked menupopup from my patch as a starting point.

> Additionally the underlying problem in bug 429469 would get a bit harder to
> fix, cause after this change some static nodes may still want the places
> context menu, while other won't.

I don't understand how this makes that harder to fix. I also think it's unlikely that bug 429469 will become a priority soon...

> And moreover this seems to adds the show/hide contextual menuitem to every
> single bookmark, even if it doesn't make any sense to have this option in a
> submenu, let alone the toolbar (since IIRC we reuse this context menu
> everywhere?)

No, see updateContextMenu in my patch, it takes care of that.
Attached patch patch v2 (obsolete) — Splinter Review
I did some refactoring, hopefully makes this code more approachable.
Attachment #8744306 - Attachment is obsolete: true
Attachment #8744591 - Flags: review?(mak77)
Blocks: 1248267
IMHO, I really liked the approach of supporting an int pref that encapsulates `kMaxResults` and set to 5 in firefox.js. Then the 'disable' action would set it to 0 and we need only hide the header menu item when that's the case in `_updateRecentBookmarks`...
This approach would make this bug dependent on my proposed patch in bug 1248267.
I don't think there's much value in having the number of items be a pref (although we can reconsider when there's demand), and I'm not a fan of overloading such a pref for disabling the UI.
Comment on attachment 8744591 [details] [diff] [review]
patch v2

Review of attachment 8744591 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser-places.js
@@ +1361,5 @@
>        insertionPoint: ".panel-subview-footer"
>      });
>    },
>  
> +  RECENTLY_BOOKMARKED_PREF: "browser.bookmarks.showRecentlyBookmarked",

off-hand, looks like using Preferences.jsm you could simplify and separate the management of this pref, and avoid having to get its value at every opening.
Something like:

XPCOMUtils.defineLazyGetter(BookmarkingUI, "recentlyBookmarked", () =>
  let prefs = new Preferences("browser.bookmarks");
  let store = {
    enabled: false,
    observe(subject, topic, data) {
      this.enabled = prefs.get("showRecentlyBookmarked", false);
      // Do whatever else
    },
    QueryInterface: XPCOMUtils.generateQI([
      Ci.nsIObserver,
      Ci.nsISupportsWeakReference
    ])
  };
  store.enabled = prefs.get("showRecentlyBookmarked", false);
  prefs.observe("showRecentlyBookmarked", store);
  return store;
});

and then just use this.recentlyBookmarked.enabled all around. Should make some code paths more readable since it decouples it from the popup opening listeners.
You could also store extraCSSClass into it and then you could avoid passing it all around.

@@ +1379,5 @@
> +      this._populateRecentBookmarks(aHeaderItem, aExtraCSSClass);
> +    }
> +
> +    if (!aListenersAdded) {
> +      this._addRecentBookmarksListeners(aHeaderItem, aExtraCSSClass);

I'd prefer this call to happen outside the method only when needed, rather than passing a blind bool, so _updateRecentBookmarks(...);_addRecentBookmarksListeners();

@@ +1468,5 @@
> +    placesContextMenu.addEventListener("popupshowing", onPlacesContextMenuShowing);
> +    bookmarksMenu.addEventListener("popuphidden", onBookmarksMenuHidden);
> +  },
> +
> +  showRecentlyBookmarked() {

the 2 methods could be unified in a toggleRecentlyBookmarked if the pref value is cached as suggested

::: browser/components/places/content/controller.js
@@ +560,5 @@
>     * Detects information (meta-data rules) about the current selection in the
>     * view (see _buildSelectionMetadata) and sets the visibility state for each
>     * of the menu-items in the given popup with the following rules applied:
> +   *  0) The "ignoreitem" attribute may be set to "true" for this code to treat
> +   *     the item as if it wasn't there.

"for this code to not handle this menuitem"

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +173,5 @@
>  <!ENTITY bookmarksToolbarChevron.tooltip "Show more bookmarks">
> +<!ENTITY showRecentlyBookmarked.label     "Show Recently Bookmarked">
> +<!ENTITY showRecentlyBookmarked.accesskey "R">
> +<!ENTITY hideRecentlyBookmarked.label     "Hide Recently Bookmarked">
> +<!ENTITY hideRecentlyBookmarked.accesskey "R">

R is taken by SO_R_T BY NAME on folders, I think we should avoid the confusion... H is not taken I think.
Attachment #8744591 - Flags: review?(mak77)
Attached patch patch v3Splinter Review
The pref system holds prefs in memory. Getting a value from it isn't expensive these days, the XPCOM overhead has been reduced greatly during the last decade. I don't think storing values as suggested is a useful optimization for code that isn't super performance-sensitive. Nor does it seem like a simplification to me; it adds a sizable chunk of code just to reduce a few lines.

I've made the other changes you requested.
Attachment #8744591 - Attachment is obsolete: true
Attachment #8746965 - Flags: review?(mak77)
Comment on attachment 8746965 [details] [diff] [review]
patch v3

Review of attachment 8746965 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, I'm not yet convinced, but no point in keeping this work from proceeding.
Let's see how it will evolve in Mike's patch.
Attachment #8746965 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/f98e3add979e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Recent regression, tracking for 48. 
Is this something you'd like to uplift to aurora? Have you confirmed the performance is ok?
Flags: needinfo?(dao+bmo)
No, the feature isn't present in 48.
Flags: needinfo?(dao+bmo)
Depends on: 1289659
I have reproduced this bug with Firefox nightly 47.0a1(build id:20160214030236)on
windows 7(64 bit)

I have verified this bug as fixed with Firefox beta 49.0b7(build id:20160825132718)
User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0

[bugday-20160831]
Thanks!
Status: RESOLVED → VERIFIED
Depends on: 1304617
No longer depends on: 1304617
Thank you guys for fixing this; the new option works well for me too, in Firefox 49.0.1.
Depends on: 1304617
Firefox 49.0.2 and still this problem continues.  No way to disable it.  IT IS AWFUL!!!!!!!!!!!!!!!!
(In reply to Brian Peppers from comment #69)
> Firefox 49.0.2 and still this problem continues.  No way to disable it.  IT
> IS AWFUL!!!!!!!!!!!!!!!!

What are you on about ?  Right-click any bookmark and use 'Hide recently bookmarked' 

Not rocket-science.
Depends on: 1386197
You need to log in before you can comment on or make changes to this bug.