"Most Visited", "Recently Bookmarked", and "Recent Tags" should be treated as folders

VERIFIED FIXED in Firefox 3

Status

()

VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: whimboo, Assigned: mak)

Tracking

Trunk
Firefox 3
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5) Gecko/2008032619 Firefox/3.0b5 ID:2008032619

Currently selecting any of the three default saved searches under "Smart Bookmarks", let you change the Location or set a keyword. It shouldn't be possible.

Steps to reproduce:
1. Open Library and choose "Smart Bookmarks" within the left pane
2. Select "Recent Tags"

Now the details pane shows the query string for that saved search, which can be modified. You can even set a keyword. Using this keyword later results in an alert message:

Firefox doesn't know how to open this address, because the protocol (place) isn't associated with any program.

Comment 1

11 years ago
Keywords I could care less about (though not by much), but I hope the query in the location box can stay exposed, un-modifiable if need be. Else-wise discoverability, customization and the tinkering experience would seem to be severely degraded.
Lets ask for blocking-firefox3 to get a clear statement from drivers.
Flags: blocking-firefox3?
This makes a lot of sense.

My take is that exposing place: URIs isn't useful, if you want to use place, read the docs or build an extension.
Assignee: nobody → mak77
Flags: blocking-firefox3? → blocking-firefox3+
(Assignee)

Comment 4

11 years ago
what if a user wants to change the max number of records returned by those queries?
Flags: blocking-firefox3+ → blocking-firefox3?
Uh, was this fixed by something else? I'm not seeing locations for smart bookmarks anymore.

Marco: I think we need to rely on add-ons and future work to help people build queries, not poorly supported UI which can easily be messed up in ways that are damaging.

Connor actually meant to minus this, despite his comment and action to the contrary. Doesn't block, might actually be fixed, very confusing!
Flags: blocking-firefox3? → blocking-firefox3-
(Assignee)

Comment 6

11 years ago
(In reply to comment #5)
> Uh, was this fixed by something else? I'm not seeing locations for smart
> bookmarks anymore.

Mike, i see them...
As of the landing of bug 421483, the context menu for "Properties" is missing for the "Most Visited" folder, making it very difficult for me to rename the folder. MaK77 showed me how I could get to it in the Library to rename it, but it would be nice to have the Properties option back, as other folders offer this option.
(Assignee)

Comment 8

11 years ago
Created attachment 315094 [details] [diff] [review]
patch

enable properties context menu for queries (but not for generated queries without an id, like date containers).
allow only name edit for queries (both in Library and in Properties dialog), hide other fields.
disable properties for folder shortcuts (we are not supporting them actually and show Edit properties for "(null)" :( plus we should not allow editing name of roots folder shortcuts)
Attachment #315094 - Flags: review?(mano)
(Assignee)

Comment 9

11 years ago
Created attachment 315097 [details] [diff] [review]
patch

better, consider them bookmarks in both panels
Attachment #315094 - Attachment is obsolete: true
Attachment #315097 - Flags: review?(mano)
Attachment #315094 - Flags: review?(mano)
Assignee: mak77 → mano
Whiteboard: [patch on bug 407541]
Oh, hrm, can you update this patch to only fix the bookmark properties dialog once i land bug 407541?
Assignee: mano → mak77
Whiteboard: [patch on bug 407541]
(Assignee)

Comment 11

11 years ago
will see what will be left after that check-in
Look good. Selecting any of the mentioned folders only shows the name and description. The user doesn't have the opportunity to change the location anymore.

Which patch fixed this issue?
Target Milestone: --- → Firefox 3
Bug 407541, we still need to fix the old dialog though.
Target Milestone: Firefox 3 → ---
(Assignee)

Updated

11 years ago
Attachment #315097 - Attachment is obsolete: true
Attachment #315097 - Flags: review?(mano)
(Assignee)

Comment 14

11 years ago
this should be at least wanted+ to be consistent with the Library details pane
Whiteboard: [needs new patch]
(Assignee)

Comment 15

11 years ago
Created attachment 317695 [details] [diff] [review]
patch

what this patch contains:

- fixed title field for tag containers (we need the concreteItemId)
- changed "load in sidebar" to "loadInSidebar" to be consistent with editOverlay
- unified _folderId, _bookmarkId to _itemId (unique identifier)
- changed _bookmarkURI to _uri to be consistent with editOverlay
- changed _forceHideRows to work more like showHideRows of editOverlay, now bookmarkProperties can show/hide wisely based on content (now renamed to _showHideRows)
- removed duped show/hide code from _populateProperties
- _showHideRows is called directly from _populateProperties
- re-enabled Properties context menuitem for queries on toolbar/menus
- removed no more used code from places.js
- unified showFolderProperties and showBookmarkProperties to showItemProperties

this work should make bookmarkProperties more similar to editOverlay, making it easier to maintain
Attachment #317695 - Flags: review?(mano)
(Assignee)

Comment 16

11 years ago
reasking blocking (wanted, really), see comment #14
Status: NEW → ASSIGNED
Flags: blocking-firefox3- → blocking-firefox3?
Whiteboard: [needs new patch] → [has patch][needs review mano]
(Assignee)

Comment 17

11 years ago
Created attachment 317698 [details] [diff] [review]
patch

fixed minor glitch in showItemProperties method
Attachment #317695 - Attachment is obsolete: true
Attachment #317698 - Flags: review?(mano)
Attachment #317695 - Flags: review?(mano)
(Assignee)

Comment 19

11 years ago
Comment on attachment 317698 [details] [diff] [review]
patch

asking approval for consistency between edit bookmarks panes and dialogs
Attachment #317698 - Flags: approval1.9?
Whiteboard: [has patch][needs review mano] → [has patch][has reviews]
Comment on attachment 317698 [details] [diff] [review]
patch

a=mconnor on behalf of 1.9 drivers
Attachment #317698 - Flags: approval1.9? → approval1.9+
Flags: blocking-firefox3? → blocking-firefox3-
(Assignee)

Updated

11 years ago
Keywords: checkin-needed
Created attachment 318120 [details] [diff] [review]
unbitrot

Fixes minor conflict with the patch from bug 430213.
Attachment #317698 - Attachment is obsolete: true
mozilla/browser/components/places/content/bookmarkProperties.js 	1.86
mozilla/browser/components/places/content/controller.js 	1.233
mozilla/browser/components/places/content/places.js 	1.162
mozilla/browser/components/places/content/placesOverlay.xul 	1.36
mozilla/browser/components/places/content/utils.js 	1.140 
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews]
Target Milestone: --- → Firefox 3
(Assignee)

Comment 23

11 years ago
thank you Gavin for the unbitrot
(Assignee)

Updated

11 years ago
Blocks: 431150
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042907 Minefield/3.0pre
Status: RESOLVED → VERIFIED
(Assignee)

Updated

11 years ago
Depends on: 431231
(Assignee)

Updated

11 years ago
Depends on: 431594
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.