make column changes in places/library (session) persistent

VERIFIED FIXED in Firefox 3 beta5

Status

()

P3
minor
VERIFIED FIXED
11 years ago
9 years ago

People

(Reporter: xtc4uall, Assigned: mak)

Tracking

Trunk
Firefox 3 beta5
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fx2-parity])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

11 years ago
STR:
- open places/library
- notice the given columns "name", "tags", "location"
- add one (or more) columns (e.g. "keyword")
- close the window and/or firefox in total

expected:
after reopening places/library the above chosen "extra-column" (e.g. "keyword") should show up.

expected_deluxe:
after restarting Firefox *and* places/library the above chosen "extra-column" (e.g. "keyword") should show up.

actual result:
the chosen column is gone
Flags: blocking-firefox3?
(Reporter)

Comment 1

11 years ago
adding Fx2 paritiy as the first mentioned non-deluxe-expectation is working with FF2.
Whiteboard: [Fx2-parity]

Updated

11 years ago
Assignee: nobody → mano
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
Target Milestone: Firefox 3 → Firefox 3 M10
Duplicate of this bug: 405200
(Reporter)

Updated

11 years ago
Duplicate of this bug: 381764
Target Milestone: Firefox 3 M10 → Firefox 3 M11
(Assignee)

Updated

11 years ago
Duplicate of this bug: 408280
Duplicate of this bug: 410224
Target Milestone: Firefox 3 beta3 → ---
(Assignee)

Updated

11 years ago
Duplicate of this bug: 418770
Marco, can you whip up a patch for this?  Should be reasonably easy to persist the columns, unless I'm missing something.
Assignee: mano → mak77
(Assignee)

Comment 8

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

this patch makes the following changes:

- columns are draggable (Bug 405017)
- column position and visibility is persistent (Bug 405010)
- column sorting is persistent (Bug 405108)
- last selected folder in the left pane is persistent (Bug 329853)

I notice a problem however: in the Library we allow to D&D only if the view is not sorted. If we persist the sorting status across sessions we will most probably have complain from the users that the D&D is not working. 

IMHO, we should persist the sorting order in the Library when a user moves through folders, but not persist it across sessions, this way when the user opens the Library the D&D is correctly working, and he will soon understand the difference between a sorted view and an unsorted one.

There already a related bug that i have still to fill, in a sorted view the drop should not be allowed (while it appear possible atm, a regression?).

Mano what do you think about this?
(Assignee)

Comment 9

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

mano i'm asking review to have your suggestions on this, see the previous comment (if you also have hints on the changes it would be great)
Attachment #306530 - Flags: review?(mano)
Do we also disable d&d in Fx2 in this case? I believe we should allow d&d to sub folders even when the view is sorted.

That said, I don't think there's much value in persisting the sort method anyway. If we cannot get sensible d&d support when the view is sorted, it'd be fine not to persist sorting for the time being.
I think you should take out "- last selected folder in the left pane is persistent (Bug 329853) " for now, this doesn't play so well with "Show All Bookmarks" and "Show All History".
(Assignee)

Comment 12

11 years ago
(In reply to comment #10)
> Do we also disable d&d in Fx2 in this case? I believe we should allow d&d to
> sub folders even when the view is sorted.

it's bogus there too, the drop is done, then the view gest sorted again (so the dropped item go to another position). Probably we should allow dropping from a folder to another folder (dropping at the end), but not moving item into the same folder since that is not reliable. Or we can retain the FX2 bogus behaviour.

> That said, I don't think there's much value in persisting the sort method
> anyway. If we cannot get sensible d&d support when the view is sorted, it'd be
> fine not to persist sorting for the time being.

i agree to not retain that across sessions, but in the same session a user visit a folder and set location sorting. Then he moves on another folder and has to set sorting again. In FX2 we are retaining sorting in this case.

> I think you should take out "- last selected folder in the left pane is
> persistent (Bug 329853) " for now, this doesn't play so well with "Show All
> Bookmarks" and "Show All History".

It should not have any problem with them, since it gives priority to window arguments, has a default to AllBookmarks, can accept both query identifers (names) than item ids. So when you click Show All bookmarks the position is remembered (except when the position is invalid, so it go back to All Bookmarks), while when you click Show All history the "history" argument takes priority over remembered one. FX2 remember last position in bookmarks organizer.
(Assignee)

Comment 13

11 years ago
(In reply to comment #11)
mmh rethinking i see what you say here, Show All Bookmarks should always go to All Bookmarks... we don't have a menuitem to open the Library, as in FX2... we could add such an item in Tools menu? still the patch would be good since show all bookmarks would have an argument, open Library would not.

we could take the change but not activate the feature (leaving the argument in show all bookmarks menuitem), in future it would be easier implement persistent folder
Can you ask Alex about this? I think he wanted "show all bookmarks" to explicitly select the "all bookmarks" :) folder
(Assignee)

Comment 15

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

Alex, Show All Bookmarks should always point to "All Bookmarks" or should remember the last selected folder as in FX2? Or do we need a new menu item to open the Library to the last selected position?
Attachment #306530 - Flags: ui-review?(faaborg)
I prefer not check in this code unelss we're using it.

As for persisting sorting, the fx2 behavior isn't desired, no. It would be nice if  you could get the "allow dropping at end" behavior working.
Comment on attachment 306530 [details] [diff] [review]
patch

switching ui-r to beltzner
Attachment #306530 - Flags: ui-review?(faaborg) → ui-review?(beltzner)
(Assignee)

Comment 18

11 years ago
Created attachment 307184 [details] [diff] [review]
stripped patch

let's take it easy, this only contains:

- columns are draggable (Bug 405017)
- column position and visibility is persistent (Bug 405010)

will move other infos / patches /requests to the respective bugs
Attachment #306530 - Attachment is obsolete: true
Attachment #307184 - Flags: review?(mano)
Attachment #306530 - Flags: ui-review?(beltzner)
Attachment #306530 - Flags: review?(mano)
(Assignee)

Updated

11 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

11 years ago
Whiteboard: [Fx2-parity] → [Fx2-parity][has patch][needs review Mano]
(Assignee)

Updated

11 years ago
Keywords: checkin-needed
Whiteboard: [Fx2-parity][has patch][needs review Mano] → [Fx2-parity][has patch][has review]
hm, doesn't apply. marco, can you unrot this?
(Assignee)

Comment 21

11 years ago
Created attachment 308117 [details] [diff] [review]
unbitrot
Attachment #307184 - Attachment is obsolete: true
Checking in browser/components/places/content/places.xul;
/cvsroot/mozilla/browser/components/places/content/places.xul,v  <--  places.xul
new revision: 1.120; previous revision: 1.119
done
Checking in browser/components/places/content/treeView.js;
/cvsroot/mozilla/browser/components/places/content/treeView.js,v  <--  treeView.js
new revision: 1.53; previous revision: 1.52
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [Fx2-parity][has patch][has review] → [Fx2-parity]
Target Milestone: --- → Firefox 3 beta5
Verified fixed with:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b5pre) Gecko/2008030806 Minefield/3.0b5pre ID:2008030806

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008030804 Minefield/3.0b5pre ID:2008030804
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Test cases https://litmus.mozilla.org/show_test.cgi?id=4132 for the 3.0 test run and https://litmus.mozilla.org/show_test.cgi?id=6090 for 3.1 test run were updated with this regression bug Id.
Flags: in-litmus? → in-litmus+
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.