Dropped items don't show up in bookmarks

RESOLVED WORKSFORME

Status

()

P2
normal
RESOLVED WORKSFORME
18 years ago
10 years ago

People

(Reporter: hyatt, Assigned: waterson)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rtm-])

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
Most of the time I do drag and drop in bookmarks the dropped bookmark doesn't
show up.  YOu even have to close and open the parent node to get the bookmark to
show up. This makes drag and drop pretty much useless.
(Reporter)

Comment 1

18 years ago
Nominating for RTM.
Status: NEW → ASSIGNED
Keywords: regression, rtm

Comment 2

18 years ago
The condition seems to be (at least to me) when dragging a bookmark or folder
from a "peer" level down a level into a folder. 

Dragging a bookmark or folder from one folder to another does work, and 
dragging a URL from a web page into a folder works as well.
(Reporter)

Comment 3

18 years ago
I have verified that RDF is sending me a bogus contentremoved on the item.  It
is actually telling me to remove it.  So this is a bug with bookmarks.
Assignee: hyatt → waterson
Status: ASSIGNED → NEW
(Assignee)

Comment 4

18 years ago
Ok, I'll take a look.

jrgm: there is at least one dup of this bug that I filed.

Comment 5

18 years ago
Actually the steps to reproduce from the top of bug 55909 also produce this
"missing bookmark" behaviour. Is that the one you're thinking of?

(Sorry, I was focussing more on the LazyRowCreator crash aspect of that bug
than what would happen subsequently).
(Assignee)

Comment 6

18 years ago
Yes, that's probably the bug I was thinking of.
(Assignee)

Comment 7

18 years ago
Wow, this has never worked. I'll attach a patch that makes it go.
Status: NEW → ASSIGNED
Keywords: regression
(Assignee)

Comment 8

18 years ago
Created attachment 17300 [details] [diff] [review]
on RemoveMember() if *directly* contains the member we want
(Reporter)

Comment 9

18 years ago
Love to a=.  I see how the new code works.  What was the old routine doing wrong?
(Assignee)

Comment 10

18 years ago
The problem was that nsXULTemplateBuilder::RemoveMember() was being too liberal
with the elements that it removed from a container. Specifically, it would
remove the element corresponding to aMember if it could trace the content model
back to aContainerElement, regardless of depth. For example,

  <tree id="foo">
     <treechildren>
        <treeitem id="bar">
           <treechildren>
              <treeitem id="baz"/>
           </treechildren>
        </treeitem>
        <treeitem id="baz" /> <!-- note the dup -->
      </treechildren>
    </tree>

If we called RemoveMember("foo", "baz"), we'd remove *both* "baz" <treeitem>s.
This is exactly what happens when we drag a bookmark into a nested folder...

  1. Add "baz" to "bar"
  2. Remove "baz" from "foo"

Anyway, this makes sure that the template builder can handle this case. A
simpler fix (read: more likely to get approved by PDT) might be to re-order the
drag & drop operations to avoid getting two "baz's" in the tree at the same time.

rjc, how hard would that be to do?
OS: Windows 2000 → All
Hardware: PC → All
(Reporter)

Comment 11

18 years ago
Ah, I see.

a=hyatt on the patch.

We *HAVE* to get this in the branch.  Bookmark management is virtually
impossible without this, since drag and drop is the only way we have of filing
bookmarks.

This exact scenario is also the *only* way you can add links to your personal
toolbar, which makes it doubly important.
Great to see this working;  sr=rjc

(Assignee)

Comment 13

18 years ago
yo scottip, could you try this patch out with threaded view and see if you
notice anything weird happening?
(Assignee)

Comment 14

18 years ago
Runnin' it up the flagpole...
Priority: P3 → P2
Whiteboard: [rtm+]
Target Milestone: --- → mozilla0.6
(Assignee)

Comment 15

18 years ago
Fix checked into trunk.
Whiteboard: [rtm+] → [rtm+] FIXED ON TRUNK

Comment 16

18 years ago
PDT says rtm need info: please land on trunk and test with threaded view as
Waterson asks, note any regressions here.
Whiteboard: [rtm+] FIXED ON TRUNK → [rtm need info]

Updated

18 years ago
Whiteboard: [rtm need info] → [rtm need info] FIXED ON TRUNK

Comment 17

18 years ago
Byte really nailed us in a review for not having this feature working.

If this didn't cause any regressions, then this should go to rtm+ immediately.

Comment 18

18 years ago
Sent to Chris Waterson <waterson@netscape.com>; he suggested this information be
added to this bug report.

B (brendan@zen.org)

>> FYI, I've been encountering problems where mailnews will bail on an 
>> attempt
>> to delete any message.   The problem is caused by 
>> IsDirectlyContainedBy endng up in a call to 
>> nsCOMPtr_base::~nsCOMPtr_base, leading to a failure inside the linux 
>> main_arena libc routine (aka, past the usual barriers).
>> 
>> I wanted to let you know, since presumably it just means that the parents
>> are being climbed up too far.  This is on a system running RH6.1 with 
>> glibc 2.1.2.  Mozilla was built with gcc 2.95.2 using the config options
>> --enable-optimize --disable-tests --disable-debug

Comment 19

18 years ago
why is this still languishing in [rtm need info] land?

Comment 20

18 years ago
This might be the cause of http://bugscape/show_bug.cgi?id=3035
I'll try out the patch. This might need to go into the branch if that's the case.

Comment 21

18 years ago
Joe: Chris has checked the patch for this into the trunk. But, I tried a trunk 
build from this morning, and when I add the buddy to the list, it is not
updated in that list. Scrolling down to hide then reveal the list, or 
collapsing and expanding the folder will cause the row to display the correct
contents.

Hyatt has a patch (in 3 parts) attached to bug 57139, that may address this 
issue. Please try them out in a commercial build, and see if this helps this
situation. Thanks. (I have these patches in a mozilla build, but I do not have
a commercial build. Sorry -- need to change that).

Comment 22

18 years ago
did fixing bug 57139 fix this bug?
(Assignee)

Updated

18 years ago
Whiteboard: [rtm need info] FIXED ON TRUNK → [rtm-] FIXED ON TRUNK
(Assignee)

Comment 23

18 years ago
Marking fixed, since it has been fixed on the trunk; I should've done this when
marking rtm-.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 24

18 years ago
Not so fast!  I'm running nightly 2000111604 on Win98se and still see this bug.

My steps to repo:
1) Start Mozilla
2) Open Sidebar
3) Expand "Mozilla Project"
4) Expand both "mozilla.org" and "Developer Information"
  -- If you don't have a scrollbar yet, expand folders until you do.
5) Scroll down so the scrollbar is NOT at the top of the scroll area.
  -- Without this step, things seem to work ok.  ie. If the top of the tree is
visible, this bug doesn't occur.
6) Drag something (bookmark or folder) so that it should appear as the first
child of a folder.

Actual Result:
   The dragged object disappears.
Expected Result:
   The dragged object appears as the first child of the parent folder.
Workaround:
   Close and reopen the parent folder; the child appears.

Note that sometimes repoducing this bug leads to bug 52298.  When that happens,
you have to shut off the Sidebar and turn it on again to restore the state.

This also occurs in "Manage Bookmarks".
(Assignee)

Comment 25

18 years ago
Sho' nuf. Scrollbars seem to screw stuff up.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [rtm-] FIXED ON TRUNK → [rtm-]

Comment 26

18 years ago
*** Bug 36873 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 27

18 years ago
Hey Ben, did your bookmarks re-write fix this?
Target Milestone: mozilla0.6 → mozilla0.9
(Assignee)

Comment 28

18 years ago
Moving to mozilla1.0, because at that point it's likely we'll have re-written
bookmarks to use the new outliner...
Target Milestone: mozilla0.9 → mozilla1.0

Comment 29

18 years ago
This bug seems to be fixed now.

Comment 30

18 years ago
NO!  This bug is not fixed for me in build 2001032004 win98se.
In fact, it's gotten *worse*.  This bug now causes DATA LOSS!  AARRGGG!

I now have trouble reproducing it consistantly.  It will eventually happen if
you follow the steps I gave above.  Sometimes, once a bookmark appears to be
gone, you can collapse the parent folder, reopen it, and the bookmark will
appear.  Other times, the bookmark will be gone forever.

Comment 31

18 years ago
OK, the dataloss thing seems to (soon) be taken care of in bug 68547.

But there are still refresh problems when dropping a bookmark (or folder) above
the top child of a folder.  As I mentioned earlier (2000-11-17 06:47), the
refresh problem only seens to happen when the top of the tree is not in the
view.  That still happens with build 2001041200 on win98se.

Is the new <outliner> going to be used for bookmarks, and if so, will it fix
this problem?
(Assignee)

Updated

18 years ago
Status: REOPENED → ASSIGNED

Comment 32

17 years ago
I can reproduce a bug that fits the summary, but I think it's independent...

Win98 build 2001080603:

1)Open Bookmark Manager
2)Create a Folder
3)Create or Move 3 bookmarks into Folder
4)Drag and drop second bookmark onto gap between Folder and first bookmark
(after it highlights)

Actual Results: Bookmark disappears until Folder is closed, then reopened.

Expected Results: Bookmark moves to first position within Folder, no refresh needed.

I'll turn this into a new bug if you don't want it.  Don't see a dup.

Comment 33

17 years ago
Just found the dup... bug 68547.  Hard to sift through all the similar issues. 
Sorry for the spam.

Comment 34

17 years ago
This bug seems to have morphed from the initial:
"D&D of bookmarks (to any folder and any position) - sometimes need a folder
refresh to see change"
to now something like this:
"D&D of bookmarks to top of a folder sometimes hides them until folder is
refreshed when top of bookmark list not in view"

norton@w5ac.tamu.edu, I think this is the correct bug for the symptom you
see.  bug 68547 was a dataloss bug, this bug is just a refresh problem I think.

When I talked (on 2000-11-17 06:47) about shutting off Sidebar (or Manage
Bookmarks) to refresh, I was talking about how sometimes the steps to reproduce
this bug lead to bug 52298, where shutting off the Sidebar is the workaround.

FWIW, I can still repro this bug on build 2001080703-trunk win98

Comment 35

17 years ago
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
(Assignee)

Updated

17 years ago
Target Milestone: mozilla1.0.1 → Future

Comment 36

17 years ago
Can anyoune reproduce this bug yet?

Comment 37

17 years ago
WFM with 2002060804/Linux.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago17 years ago
Resolution: --- → WORKSFORME

Updated

10 years ago
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.