Closed Bug 637835 Opened 9 years ago Closed 9 years ago

"Error: gFolderDisplay is not defined" in Advanced Search dialog when trying to drag search result entry (d&d impossible)

Categories

(SeaMonkey :: MailNews: General, defect, major)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1final

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

References

Details

(Keywords: regression)

Attachments

(1 file)

Error: gFolderDisplay is not defined
Source File: chrome://messenger/content/messengerdnd.js
Line: 312

when trying to drag a message in the Advanced Search dialog's search results.

Reason AFAICS:
1. SearchDialog.xul uses XUL overlay threadPane.xul which contains ondraggesture="BeginDragThreadPane(event);"
2. SearchDialog.xul loads SearchDialog.js before messengerdnd.js, so the latter overrides the former
Keywords: regression
In 20110503010002 build this bug is still present.

I politely ask for blocking status (of course, patch and FIXED status is enough for me) for this bug, because this is regression in existing function of GUI: drag&drop function for moving collection of founded items to another folder is not functional anymore. Remaining function under button "File" is really pain to use frequently. Thank you for considering.
blocking-seamonkey2.1: --- → ?
Raising severity; I hadn't realized that this issue totally breaks d&d.
Severity: normal → major
Summary: "Error: gFolderDisplay is not defined" in Advanced Search dialog when trying to drag search result entry → "Error: gFolderDisplay is not defined" in Advanced Search dialog when trying to drag search result entry (d&d impossible)
[Please keep in mind how close the 2.1 cut is and the regression status of this when reviewing, thanks.]

So, some further investigation shows:
* BeginDragThreadPane in SearchDialog.js is just a stub and has probably never been used (or not for a long time). We should just remove it.
* BeginDragThreadPane in messengerdnd.js was changed to use gFolderDisplay in bug 537448
* gFolderDisplay is not available in the Advanced Search window yet since folderDisplay.js is loaded through mailWindowOverlay.xul which is only included by messenger.xul and messageWindow.xul
* even if gFolderDisplay is made available (through a <script> line in SearchDialog.xul), BeginDragThreadPane in messengerdnd.js will return early since gFolderDisplay needs gDBView to work properly.

So what needs to be done is this:
1. include folderDisplay.js
2. rename gSearchView to gDBView
3. include contentAreaUtils.js for validateFileName() which is used in suggestUniqueFileName() in utilityOverlay.js which is called from BeginDragThreadPane() in messengerdnd.js
4. rename gCurrentFolder to gMsgFolderSelected which is used in DragOverThreadPane in messengerdnd.js (also used by get displayedFolder(), so using gFolderDisplay instead doesn't buy us anything in the short run)
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #530059 - Flags: superreview?(neil)
Attachment #530059 - Flags: review?(mnyromyr)
Can't we just switch back to GetSelectedMessages()?
(In reply to comment #4)
> Can't we just switch back to GetSelectedMessages()?

I guess we could, but that would only allow for skipping points (1) and (2) from comment 3 and had these disadvantages:
* we won't be able to use gFolderDisplay in any of those d&d functions in the future due to the Search window dependency, which is not obvious (messengerdnd.js is mostly used for the normal windows which do have support for gFolderDisplay)
* the Search window will be the exception in that it doesn't support gFolderDisplay, but only for SM (TB uses it, but their actual implementation is quite different there). Also this seems backwards to me.
* we'll have to rename gCurrentFolder anyway, which will bring us closer to the situation of the other windows. Keeping gSearchView will add more confusion to what is available where and what is not.

Another option, if you really dislike the renaming of gSearchView to gDBView, would be to change folderDisplay.js to call GetDBView() (maybe with an internal getter that uses a fallback like GetDBView ? GetDBView() : gDBView, if we're unsure whether GetDBView is always available).
Comment on attachment 530059 [details] [diff] [review]
patch [Checkin: comment 7]

OK I'm sold.
Attachment #530059 - Flags: superreview?(neil) → superreview+
Attachment #530059 - Flags: review?(mnyromyr) → review+
Status: ASSIGNED → RESOLVED
blocking-seamonkey2.1: ? → ---
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1final
Depends on: 660882
Blocks: 660882
No longer depends on: 660882
You need to log in before you can comment on or make changes to this bug.