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)
Firefox
Bookmarks & History
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)
9.30 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•8 years ago
|
||
Needinfo verdi to answer the above questions.
Flags: needinfo?(mverdi)
Comment 2•8 years ago
|
||
(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)
Assignee | ||
Comment 3•8 years ago
|
||
(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)
Comment hidden (off-topic) |
Comment 5•8 years ago
|
||
(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)
Updated•8 years ago
|
Summary: Stop adding the "Recently bookmarked" smart folder → Stop adding the "Recently bookmarked" smart folder for new profiles
Comment 6•8 years ago
|
||
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.
Comment 7•8 years ago
|
||
(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.
Assignee | ||
Comment 8•8 years ago
|
||
Comment 9•8 years ago
|
||
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+
Comment 11•8 years ago
|
||
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)
Comment 12•8 years ago
|
||
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
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(dao)
Comment 14•8 years ago
|
||
Backed out for XPCshell failure in test_browserGlue_smartBookmarks.js. r=backout Backout: https://hg.mozilla.org/integration/fx-team/rev/870749af34d8 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=c6d163cfa08f06bd41350bd236713092e732159c Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=8600033&repo=fx-team
Flags: needinfo?(dao)
Comment 15•8 years ago
|
||
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
Assignee | ||
Comment 16•8 years ago
|
||
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 17•8 years ago
|
||
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+
Assignee | ||
Comment 18•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=20a60baca4d6
Comment 20•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e2a8cafd7d2e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment 21•8 years ago
|
||
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?
Comment 22•8 years ago
|
||
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.
Comment 23•8 years ago
|
||
(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.
Comment 24•8 years ago
|
||
(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.
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•