Closed Bug 425851 Opened 12 years ago Closed 12 years ago

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

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: whimboo, Assigned: mak)

References

Details

Attachments

(1 file, 4 obsolete files)

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.
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+
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-
(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.
Attached patch patch (obsolete) — Splinter Review
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)
Attached patch patch (obsolete) — Splinter Review
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]
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 → ---
Attachment #315097 - Attachment is obsolete: true
Attachment #315097 - Flags: review?(mano)
this should be at least wanted+ to be consistent with the Library details pane
Whiteboard: [needs new patch]
Attached patch patch (obsolete) — Splinter Review
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)
reasking blocking (wanted, really), see comment #14
Status: NEW → ASSIGNED
Flags: blocking-firefox3- → blocking-firefox3?
Whiteboard: [needs new patch] → [has patch][needs review mano]
Attached patch patch (obsolete) — Splinter Review
fixed minor glitch in showItemProperties method
Attachment #317695 - Attachment is obsolete: true
Attachment #317698 - Flags: review?(mano)
Attachment #317695 - Flags: review?(mano)
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-
Keywords: checkin-needed
Attached patch unbitrotSplinter Review
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
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews]
Target Milestone: --- → Firefox 3
thank you Gavin for the unbitrot
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
Depends on: 431231
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.