Closed Bug 56349 Opened 24 years ago Closed 22 years ago

Dropped items don't show up in bookmarks

Categories

(Core :: XUL, defect, P2)

Product:
Core
Component:
XUL
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME
Milestone:
Future

People

(Reporter: hyatt, Assigned: waterson)

References

Details

(Whiteboard: [rtm-])

Attachments

(1 file)

on RemoveMember() if *directly* contains the member we want
3.48 KB, patch
Details | Diff | Splinter Review 
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.
Nominating for RTM.
Status: NEW → ASSIGNED
Keywords: regression, rtm
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.
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
Ok, I'll take a look.

jrgm: there is at least one dup of this bug that I filed.
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).
Yes, that's probably the bug I was thinking of.
Wow, this has never worked. I'll attach a patch that makes it go.
Status: NEW → ASSIGNED
Keywords: regression
Love to a=.  I see how the new code works.  What was the old routine doing wrong?

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
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


yo scottip, could you try this patch out with threaded view and see if you
notice anything weird happening?
Runnin' it up the flagpole...
Priority: P3 → P2
Whiteboard: [rtm+]
Target Milestone: --- → mozilla0.6
Fix checked into trunk.
Whiteboard: [rtm+] → [rtm+] FIXED ON TRUNK
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]
Whiteboard: [rtm need info] → [rtm need info] FIXED ON TRUNK
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.

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

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

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.

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).
did fixing bug 57139 fix this bug?
Whiteboard: [rtm need info] FIXED ON TRUNK → [rtm-] FIXED ON TRUNK
Marking fixed, since it has been fixed on the trunk; I should've done this when
marking rtm-.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
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".
Sho' nuf. Scrollbars seem to screw stuff up.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [rtm-] FIXED ON TRUNK → [rtm-]
*** Bug 36873 has been marked as a duplicate of this bug. ***
Hey Ben, did your bookmarks re-write fix this?
Target Milestone: mozilla0.6 → mozilla0.9
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
This bug seems to be fixed now.

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.
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?
Status: REOPENED → ASSIGNED
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.
Just found the dup... bug 68547.  Hard to sift through all the similar issues. 
Sorry for the spam.
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
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
Target Milestone: mozilla1.0.1 → Future
Can anyoune reproduce this bug yet?
WFM with 2002060804/Linux.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 22 years ago
Resolution: --- → WORKSFORME
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.

Attachment

General

Creator:
Created:
Updated:
Size: