Closed Bug 1248617 Opened 8 years ago Closed 8 years ago

Stop adding the "Recently bookmarked" smart folder for new profiles

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox46 --- unaffected
firefox47 --- disabled
firefox48 --- fixed

People

(Reporter: dao, Assigned: dao)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1219804 makes this smart folder redundant, so we should stop adding it. We might want to remove if from existing profiles, too. This might also be a good opportunity to decide if we want to keep the Recent Tags smart folder in the menu.
Needinfo verdi to answer the above questions.
Flags: needinfo?(mverdi)
(In reply to Dão Gottwald [:dao] from comment #0)
> Bug 1219804 makes this smart folder redundant, so we should stop adding it.
> We might want to remove if from existing profiles, too. This might also be a
> good opportunity to decide if we want to keep the Recent Tags smart folder
> in the menu.

We should stop adding the recent bookmarks smart folder. Let's talk about removing it from existing users in our meeting today. Let's keep the recent tags smart folder for now - I don't think it's hurting anything.
Flags: needinfo?(mverdi)
(In reply to Verdi [:verdi] from comment #2)
> Let's talk about removing it from existing users in our meeting today.

Has this been discussed?
Flags: needinfo?(mverdi)
(In reply to Dão Gottwald [:dao] from comment #3)
> (In reply to Verdi [:verdi] from comment #2)
> > Let's talk about removing it from existing users in our meeting today.
> 
> Has this been discussed?

In the meeting we decided that we should leave the smart folder in place for existing users and stop adding it for new users.
Flags: needinfo?(mverdi)
Summary: Stop adding the "Recently bookmarked" smart folder → Stop adding the "Recently bookmarked" smart folder for new profiles
You need to tell old users how to get rid of "recently bookmarked" everywhere. It is a horrendous pain. First, the list it contains has little to do with pages being "bookmarked". Many things there were just visited and never (intentionally) bookmarked. Second, if you use alt-b followed by the first letter of some commonly used bookmark (something I've been doing for years, so that it is automatic) in order to access commonly used bookmarks, if frequently goes to something in "Recently Bookmarked". I can remove "Recently Bookmarked" from the bookmark sidebar, but it is still there in the drop-down menu from alt-b.
(In reply to Jonathan Baron from comment #6)
> You need to tell old users how to get rid of "recently bookmarked"
> everywhere.

This bug is only about the old recently bookmarked query, not the items inlined in the bookmarks menu itself.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #8738997 - Flags: review?(mak77)
Comment on attachment 8738997 [details] [diff] [review]
patch

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

::: browser/components/nsBrowserGlue.js
@@ +2254,5 @@
>            url: "place:sort=" + queryOptions.SORT_BY_VISITCOUNT_DESCENDING +
>                      "&maxResults=" + MAX_RESULTS,
>            parentGuid: PlacesUtils.bookmarks.toolbarGuid,
>            newInVersion: 1
>          },

please bump const SMART_BOOKMARKS_VERSION = 7; so we will remove the smart bookmark annotation from the removed query in existing profiles.
Attachment #8738997 - Flags: review?(mak77) → review+
Backed this out because it likely causes https://treeherder.mozilla.org/logviewer.html#?job_id=8537004&repo=fx-team

Backout: https://hg.mozilla.org/integration/fx-team/rev/6fab7914b572
Failure: 08:48:57     INFO -  1362 INFO TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_bookmarks.html | Bookmark has expected index - Expected: 4, Actual:
Flags: needinfo?(dao)
I'm not sure about the ext test, probably it just hardcodes number of default bookmarks, though we should also probably change SMART_BOOKMARKS_ON_MENU http://mxr.mozilla.org/mozilla-central/source/browser/components/places/tests/unit/head_bookmarks.js#43
Flags: needinfo?(dao)
08:07:53     INFO -  TEST-START | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js
08:07:56  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | xpcshell return code: 0
08:07:56     INFO -  TEST-INFO took 2877ms
08:07:56     INFO -  >>>>>>>
08:07:56     INFO -  PROCESS | 7944 | [7944] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2667
08:07:56     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
08:07:56     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
08:07:56     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
08:07:56     INFO -  running event loop
08:07:56     INFO -  browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | Starting setup
08:07:56     INFO -  (xpcshell/head.js) | test setup pending (2)
08:07:56     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
08:07:56     INFO -  PROCESS | 7944 | [7944] WARNING: Failed to retarget HTML data delivery to the parser thread.: file c:/builds/moz2_slave/fx-team-w32-d-0000000000000000/build/src/parser/html/nsHtml5StreamParser.cpp, line 967
08:07:56     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | setup - [setup : 49] true == true
08:07:56     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | setup - [setup : 50] true == true
08:07:56     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | setup - [setup : 51] ..
08:07:56     INFO -  (xpcshell/head.js) | test run_next_test 1 pending (2)
08:07:56     INFO -  (xpcshell/head.js) | test setup finished (2)
08:07:56     INFO -  browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | Starting test_version_0
08:07:56     INFO -  (xpcshell/head.js) | test test_version_0 pending (2)
08:07:56     INFO -  "All smart bookmarks are created if smart bookmarks version is 0."
08:07:56     INFO -  (xpcshell/head.js) | test run_next_test 1 finished (2)
08:07:56     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | test_version_0 - [test_version_0 : 58] {"guid":"02J1_URKaOX5","index":0,"type":1,"dateAdded":"2016-04-12T15:07:54.529Z","lastModified":"2016-04-12T15:07:54.729Z","titl == true
08:07:56     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | test_version_0 - [test_version_0 : 63] {"guid":"RVgWF7h0P-tS","index":0,"type":1,"dateAdded":"2016-04-12T15:07:54.780Z","lastModified":"2016-04-12T15:07:54.990Z","titl == true
08:07:56     INFO -  PROCESS | 7944 | Found child(0): Most Visited
08:07:56     INFO -  PROCESS | 7944 | Found child(1): Getting Started
08:07:56     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | test_version_0 - [test_version_0 : 74] 2 == 2
08:07:56     INFO -  PROCESS | 7944 | Found child(0): Recent Tags
08:07:56     INFO -  PROCESS | 7944 | Found child(1): ---
08:07:56     INFO -  PROCESS | 7944 | Found child(2): Mozilla Firefox
08:07:56     INFO -  TEST-PASS | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | test_version_0 - [test_version_0 : 76] 3 == 3
08:07:56  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js | test_version_0 - [test_version_0 : 80] 8 == 7
08:07:56     INFO -      C:/slave/test/build/tests/xpcshell/tests/browser/components/places/tests/unit/test_browserGlue_smartBookmarks.js:test_version_0:80
08:07:56     INFO -      _run_next_test@C:\slave\test\build\tests\xpcshell\head.js:1540:9
08:07:56     INFO -      do_execute_soon/<.run@C:\slave\test\build\tests\xpcshell\head.js:692:9
08:07:56     INFO -      _do_main@C:\slave\test\build\tests\xpcshell\head.js:209:5
08:07:56     INFO -      _execute_test@C:\slave\test\build\tests\xpcshell\head.js:533:5
08:07:56     INFO -      @-e:1:1
08:07:56     INFO -  exiting test
Attached patch patch v3Splinter Review
Not sure if I'm doing the right thing in test_browserGlue_smartBookmarks.js
Attachment #8738997 - Attachment is obsolete: true
Flags: needinfo?(dao)
Attachment #8740897 - Flags: review?(mak77)
Comment on attachment 8740897 [details] [diff] [review]
patch v3

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

yes, the changes to test_browserGlue_smartBookmarks.js look ok, the test should retain its validity also after the changes thanks to the separator. It should probably be rewritten to not rely on which smart bookmarks we add/remove... btw, not important enough to be worth it at this point. I wonder how much before we remove all the smart queries (since tagging is rarely used due to sucky UI and most visited is barely usable and on an hidden by default toolbar)

Were these the only failures? Did you try a linux Try run, just in case?
Attachment #8740897 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/e2a8cafd7d2e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Any advice on how to get rid of this without starting a new profile (after years of configuring the old one)? I suppose I could start a new one, but I just wonder if I must. Is there some particular component of the new one that contains the code that was changed?
get rid of what? for the old query you can just right click and delete, for the new inline elements you can follow bug 1248268.
(In reply to Marco Bonardo [::mak] from comment #22)
> get rid of what? for the old query you can just right click and delete, for
> the new inline elements you can follow bug 1248268.

Get rid of "Recently bookmarked" in the bookmarks menu. The "bookmarks menu" is what I get by typing alt-b. I you keep the bar with "File Edit History Bookmarks ..." (which I don't, since all its functions work with alt keys) it is also what you get by clicking "Bookmarks".

I have gotten rid of "Recently bookmarked" in the bookmarks sidebar, but that did nothing to the menu.

I checked bug 1248268, where I had asked the same question, but I did not see an answer.
(In reply to Jonathan Baron from comment #23)
> I checked bug 1248268, where I had asked the same question, but I did not
> see an answer.

The answer is the bug itself, when it will be fixed it will be possible to remove it.
Depends on: 1327950
You need to log in before you can comment on or make changes to this bug.