unable to drag and drop or "Edit | Copy" then paste a container/folder from the history sidebar into the bookmarks toolbar (or bookmark organizer / sidebar) [used to crash @ BindStatementURI(), now does nothing]

RESOLVED FIXED in Firefox 3 alpha8

Status

()

Firefox
Bookmarks & History
RESOLVED FIXED
11 years ago
8 years ago

People

(Reporter: mano, Assigned: (not reading, please use seth@sspitzer.org instead))

Tracking

unspecified
Firefox 3 alpha8
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

STR:
1. Apply the patch from bug 359462 in a --enable-places-bookmarks build.
2. "View by date" in the history sidebar
3. pick a folder
4. Choose copy from the edit menu.
5. Right click the bookmarks toolbar, choose paste

We then crash in BindStatementURI (called by getItemTitle) because aURL is null.
Keywords: crash

Comment 1

11 years ago
Created attachment 252162 [details] [diff] [review]
disable copying day-folders

The day-folders don't have enough information to get their children back from the clipboard yet.  At some point I can imagine us wanting to figure out a way to express them on the clipboard, but for now, this patch just makes sure that those nodes don't get copied (and hence can't get pasted).  Comments? Other approaches?
Attachment #252162 - Flags: review?(sspitzer)
joey, once again, thanks for working on this!

questions / comments:

1)  how do we handle other containers from the history sidebar?  Like, if I'm sorted by date and site, and I choose a site folder under a date folder?  will your patch work for that?

2)  additionally, what about other containers?  what if I am in the bookmarks sidebar (which doesn't exist yet for places, except for for you and dmills, who have patches started) or the organize bookmarks dialog, or personal toolbar folder and I copy a folder?  Do we crash as well?

3)  fx 2 does not appear to support the copying of folders from the history sidebar.  the context menus don't show copy (but the edit menu has it enabled, which seems like a fx 2 bug.)  we might want to support this in the future (as you write in comment #1).  Perhaps log a spin off RFE bug on that, reference this bug, and assign to beltzner/faaborg for consideration.
Pasting "Today"'s folder as a bookmark folder seems like a valid use case to me. We should rather try to fix the copy/paste code IMO.

Comment 4

11 years ago
(In reply to comment #2)
> 1)  how do we handle other containers from the history sidebar?  Like, if I'm
> sorted by date and site, and I choose a site folder under a date folder?  will
> your patch work for that?
I imagine we'd crash there for the same reason, a lack of a folder-id, although I haven't explicitly tested this.

> 
> 2)  additionally, what about other containers?  what if I am in the bookmarks
> sidebar (which doesn't exist yet for places, except for for you and dmills, who
> have patches started) or the organize bookmarks dialog, or personal toolbar
> folder and I copy a folder?  Do we crash as well?
Copying from the organize boomarks dialog, and other bookmarks areas works fine, because we have a folder-id that we can serialize to the clipboard.

> 
> 3)  fx 2 does not appear to support the copying of folders from the history
> sidebar.  the context menus don't show copy (but the edit menu has it enabled,
> which seems like a fx 2 bug.)  we might want to support this in the future (as
> you write in comment #1).  Perhaps log a spin off RFE bug on that, reference
> this bug, and assign to beltzner/faaborg for consideration.
I'm happy to do that.

(In reply to comment #3)
> Pasting "Today"'s folder as a bookmark folder seems like a valid use case to
> me. We should rather try to fix the copy/paste code IMO.
> 
It's not even clear to me what the semantics of such a copy/paste would be.  Are we copying a 'live' folder, such that it would always contain the pages you visited today, or are we copying a new folder containing all of the pages you visited on the day you copy/pasted?
Summary: Crash when pasting a folder from the history sidebar into the bookmarks toolbar → Crash [ @ BindStatementURI ] when pasting a folder from the history sidebar into the bookmarks toolbar
still a crasher, and I have a simple one liner to bullet proof so we don't crash.

beyond that, one idea discussed for putting bookmarks / folders of bookmarks onto the clipboard would be to create a json object and serialize that to / from a string.

I'm going to attach a patcher to bullet proof against this crash, and then morph the bug to be "can't copy and paste folder from history sidebar to bookmarks"
Assignee: nobody → sspitzer
OS: Mac OS X → All
Hardware: Macintosh → All
Target Milestone: Firefox 3 alpha2 → Firefox 3 alpha6
Created attachment 268907 [details] [diff] [review]
don't crash patch
Attachment #252162 - Attachment is obsolete: true
Attachment #268907 - Flags: review?(dietrich)
Attachment #252162 - Flags: review?(sspitzer)
with that patch, we'll silently fail to paste.

on the console you'll get:

WARNING: NS_ENSURE_TRUE(aURI) failed: file c:/builds/trunk/mozilla/toolkit/compo
nents/places/src/nsNavHistory.cpp, line 4759
WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: file c:/builds/trunk/mozilla/t
oolkit/components/places/src/nsNavHistory.cpp, line 3010
JavaScript error: , line 0: uncaught exception: [Exception... "Component returne
d failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsINavHistoryService.getP
ageTitle]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS fra
me :: chrome://browser/content/places/utils.js :: anonymous :: line 505"  data:
no]
Attachment #268907 - Flags: review?(dietrich) → review+
"no crash patch" checked in, morphing bug.

Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHis
tory.cpp
new revision: 1.133; previous revision: 1.132
done

Flags: blocking-firefox3?
Summary: Crash [ @ BindStatementURI ] when pasting a folder from the history sidebar into the bookmarks toolbar → unable to paste a folder from the history sidebar into the bookmarks toolbar [used to crash @ BindStatementURI(), now does nothing]
no longer a crasher, so no longer critical.
Severity: critical → normal
Keywords: crash
Blocks: 333319
retargeting bugs that don't meet the alpha release-blocker criteria at
http://wiki.mozilla.org/Firefox3/Schedule.
Assignee: sspitzer → nobody
Target Milestone: Firefox 3 alpha6 → Firefox 3 beta1
Flags: blocking-firefox3? → blocking-firefox3+
Summary: unable to paste a folder from the history sidebar into the bookmarks toolbar [used to crash @ BindStatementURI(), now does nothing] → unable to drag and drop or "Edit | Copy" then paste a container/folder from the history sidebar into the bookmarks toolbar (or bookmark organizer / sidebar) [used to crash @ BindStatementURI(), now does nothing]
Blocks: 386267

Updated

11 years ago
Assignee: nobody → cyen

Comment 11

11 years ago
Created attachment 271774 [details] [diff] [review]
interdiff with patch for bug 378558

Depends on the patch for bug 378558, but the three lines shown in this patch still need to be added to allow DnD from History to the Bookmarks Manager.

Copy works without this patch, but because History is read-only, "copy" must be set to be true when dragging from a read-only container.
Attachment #271774 - Flags: review?(sspitzer)
Comment on attachment 271774 [details] [diff] [review]
interdiff with patch for bug 378558

two comments:

1) we can move this out of the loop

2) sourceView is null when you dnd to the toolbar (at least it was for me when I tested.)
Attachment #271774 - Flags: review?(sspitzer)

Updated

11 years ago
Depends on: 388337

Updated

11 years ago
Depends on: 388601

Updated

11 years ago
Target Milestone: Firefox 3 M7 → Firefox 3 M8

Updated

11 years ago
No longer blocks: 386267
Depends on: 386267

Updated

11 years ago
No longer depends on: 386267

Comment 13

10 years ago
So, two of the three issues outlined in the summary have been resolved:

 - this is no longer a crasher, thanks to Seth's committed patch
 - copy/paste from the history sidebar does in fact work now (though according to the discussion in bug 386267, we're not adding a UI component to make it clear visually that it's possible)
 - DnD works with the attached patch everywhere /except/ for the toolbar (see bug 388601)

so... I'm marking this bug RESOLVED|FIXED and spinning off/clarifying the DnD issue into bug 388601.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

10 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 14

10 years ago
Sorry for the bugspam, reassigning to seth before marking resolved.
Assignee: christineyen+bugs → sspitzer
Status: REOPENED → NEW

Updated

10 years ago
Status: NEW → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
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.