Closed
Bug 415043
Opened 17 years ago
Closed 17 years ago
dragging many bookmarks duplicates them and mix up selection (but on folder change they are correct)
Categories
(Firefox :: Bookmarks & History, defect)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
FIXED
Firefox 3 beta3
People
(Reporter: mak, Assigned: mak)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
asaf
:
review+
mtschrep
:
approval1.9b3+
|
Details | Diff | Splinter Review |
While working on selection i found this: STR: - open the Library and go to a folder in the bookmarks menu - suppose you have bookmarks a,b,c,d,e,f,g,h,i - select b,c and drag them between f and g - you end up with a,c,e,f,b,c,g,h,i and selection is messed up - if you change folder and come back the results are correct this is mostly an itemMoved problem this should be blocking, makes reordering bookmarks an hell i've tested this patch and it's working fine with bookmarks and folders (open and closed). notice that the problem is visible in library but could be also in bookmarks sidebar
Flags: blocking-firefox3?
Attachment #300620 -
Flags: review?(mano)
Assignee | ||
Comment 1•17 years ago
|
||
note: there is also a problem if you drag items in the white space under the tree, that regressed with Bug 412988 check-in, index becomes index -1, but it's already correct here, so the item is showed one position upper.
Comment 2•17 years ago
|
||
Comment on attachment 300620 [details] [diff] [review] patch itemRemoved doesn't always change the selection (think of a tree opened in a background window with some other selection in the same folder).
Attachment #300620 -
Flags: review?(mano) → review-
Assignee | ||
Comment 3•17 years ago
|
||
Comment on attachment 300620 [details] [diff] [review] patch to make things clearer the main problem here is not the selection itself (minor issue) but that the code that recalculates the items to be removed is wrong, so you end up with visible (but not existant really) dupes. after a chat with mano i'm going to make a new patch partially duplicating itemRemoved code to make a correct removal without touching up the selection
Attachment #300620 -
Attachment is obsolete: true
Assignee | ||
Comment 4•17 years ago
|
||
instead of duplicating code i preferred to create a new utility function, that should be more mantainable (in future if you would have to fix itemRemoved you should also have to fix itemMoved). what this patch do: - add an internal method _fixViewIndexOnRemove to treeView.js, this contains the code that was previously used in itemRemoved, and is now used in itemMoved too (sanitized) - fix itemMoved and itemRemoved to use the new method (cleaned up no more used variables) notice: this fixed mixed-up selection too! i've tested dragging and removing in different views in Library and sidebar and i've not found any problem for now
Attachment #300870 -
Flags: review?(mano)
Assignee | ||
Comment 5•17 years ago
|
||
Comment on attachment 300870 [details] [diff] [review] updated patch clearing review by mano's request, coming soon with a new patch
Attachment #300870 -
Flags: review?(mano)
Assignee | ||
Comment 6•17 years ago
|
||
added comments on return value Mano: is this ok or should i add something more?
Attachment #300870 -
Attachment is obsolete: true
Attachment #300957 -
Flags: review?(mano)
Assignee | ||
Updated•17 years ago
|
Attachment #300957 -
Attachment is obsolete: true
Attachment #300957 -
Flags: review?(mano)
Assignee | ||
Updated•17 years ago
|
Attachment #300964 -
Flags: review?(mano)
Comment 8•17 years ago
|
||
Comment on attachment 300964 [details] [diff] [review] Updated patch r=mano.
Attachment #300964 -
Flags: review?(mano)
Attachment #300964 -
Flags: review+
Attachment #300964 -
Flags: approval1.9b3?
Updated•17 years ago
|
Attachment #300964 -
Flags: approval1.9b3? → approval1.9b3+
Updated•17 years ago
|
Keywords: checkin-needed
Comment 9•17 years ago
|
||
Checking in browser/components/places/content/treeView.js; /cvsroot/mozilla/browser/components/places/content/treeView.js,v <-- treeView.js new revision: 1.35; previous revision: 1.34 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 beta3
Assignee | ||
Updated•17 years ago
|
Flags: blocking-firefox3?
Comment 10•15 years ago
|
||
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.
Description
•