Last Comment Bug 737115 - Remove deprecated dnd methods from feed subscribe dialog, several enhancements
: Remove deprecated dnd methods from feed subscribe dialog, several enhancements
Product: MailNews Core
Classification: Components
Component: Feed Reader (show other bugs)
: unspecified
: All All
-- normal (vote)
: Thunderbird 14.0
Assigned To: alta88
: 272473 (view as bug list)
Depends on:
Blocks: 795002 444813
  Show dependency treegraph
Reported: 2012-03-19 11:58 PDT by alta88
Modified: 2012-09-27 10:03 PDT (History)
3 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch. (31.32 KB, patch)
2012-03-19 12:07 PDT, alta88
no flags Details | Diff | Splinter Review
patch. (30.93 KB, patch)
2012-03-21 10:00 PDT, alta88
mozilla: review+
Details | Diff | Splinter Review
unbitrotted patch for checkin. (31.28 KB, patch)
2012-03-28 07:02 PDT, alta88
alta88: review+
Details | Diff | Splinter Review

Description User image alta88 2012-03-19 11:58:36 PDT

Comment 1 User image alta88 2012-03-19 12:07:15 PDT
Created attachment 607246 [details] [diff] [review]

1. remove com interfaces and nsDragAndDrop in favor of getting everything from the event.dataTransfer.
2. allow copy of feeds to other accounts.
3. try harder to get valid urls; support dnd from chrome/safari links (ie8/9 dnd doesn't have the info in dt though).  for both folderpane and dialog.
Comment 2 User image alta88 2012-03-19 12:11:36 PDT
*** Bug 272473 has been marked as a duplicate of this bug. ***
Comment 3 User image alta88 2012-03-21 10:00:25 PDT
Created attachment 607995 [details] [diff] [review]

tweak to make a reusable func.
Comment 4 User image David :Bienvenu 2012-03-27 16:37:13 PDT
Comment on attachment 607995 [details] [diff] [review]

this seems to work ok. My one question is about imap - I was able to drag an rss folder to an imap account once, and the messages were copied, but then I couldn't actually read the messages. When I tried to recreate the issue, I was unable to copy the rss feed to an imap account because creating the folder failed because of hierarchy delimiter issues (we were using ^ instead of . for some reason). The code w/o your patch failed the same way, so I'm not going to minus the patch for that. But, would you expect feeds to work if they're in imap accounts? Is drag&drop of feeds to imap accounts useful?
Comment 5 User image David :Bienvenu 2012-03-27 16:40:29 PDT
oh, I should say, the patch has bit-rotted a little bit - feed-subscriptions.xul doesn't quite apply.
Comment 6 User image alta88 2012-03-28 07:02:33 PDT
Created attachment 610122 [details] [diff] [review]
unbitrotted patch for checkin.
Comment 7 User image alta88 2012-03-28 07:12:47 PDT
(In reply to David :Bienvenu from comment #4)
> Comment on attachment 607995 [details] [diff] [review]
> patch.
> But, would you expect feeds to work
> if they're in imap accounts? Is drag&drop of feeds to imap accounts useful?

currently a message is a 'feed' only if its folder.server.type == "rss", otherwise it's subject to remote content and the web page won't be loaded.  but, the mbox (summary) should be viewable no differently in imap than in any other server type, ie local folders etc.

'feedness' should be a property of the message not its folder, bug 522645.  now that bug 647699 has been recently fixed, a property can be added to do this, going forward.
Comment 8 User image alta88 2012-03-28 07:17:29 PDT
btw, the 'copy feeds to other accounts' meant the feed subscription in the Subscribe dialog (not folder/messages in folderpane).

the folderpane is extremely confusing, even to me.  a 'feed' is not the folder.  a folder in a feed account may have 0, 1, or more than 1 feed subscriptions (just a folder property).  and the user can't tell without looking in the Subscribe dialog.
Comment 9 User image Ryan VanderMeulen [:RyanVM] 2012-03-28 17:37:11 PDT

BTW, I had a lot of difficulty getting your patch to apply due to bad hunks. Please make sure you're using the settings shown in the article below for the patches you submit.

Note You need to log in before you can comment on or make changes to this bug.