Closed Bug 1319299 Opened 8 years ago Closed 8 years ago

Empty mails from year 1970 created when moving mails between local folders using the "Move To" button on the "Search messages" panel (porting SeaMonkey Bug 1133212 to Thunderbird is needed)

Categories

(Thunderbird :: Search, defect)

45 Branch
defect
Not set
major

Tracking

(thunderbird_esr4551+ fixed, thunderbird51 fixed, thunderbird52 fixed, thunderbird53 fixed)

RESOLVED FIXED
Thunderbird 53.0
Tracking Status
thunderbird_esr45 51+ fixed
thunderbird51 --- fixed
thunderbird52 --- fixed
thunderbird53 --- fixed

People

(Reporter: ddegan, Assigned: jorgk-bmo)

References

(Depends on 1 open bug)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161104212021

Steps to reproduce:

Windows 7 family
Thunderbird 45.5 but this problem has been persistant for at least 18 months

Using Edition/Search in the mail. Any kind of search (in title, sender, etc.)
I find some messages (any number of messages).
I send them to another box (any other box).


Actual results:

The message is sent to the other box (which is expected)
An empty message, year 1970, is created in this box, tagged "non read"
If I move 10 messages, I'll have 10 empty messages (+ the 10 real messages that I actually wanted to move)


Expected results:

The message is sent to the other box and no empty message is created.
What happens if you repair the target folder? Right-click folder, Properties, Repair.
What do you mean by "Advanced Search"? The Global Search and Indexer (Gloda) accessible via <ctrl>K?

Or you me "normal" search accessible via <ctrl><shift>F?
Edit > Find > Search Messages.

I tried both and I cannot reproduce your problem. The found messages are in a local or IMAP folder? And they are moved to a local or IMAP folder?
(In reply to Jorg K (GMT+1) from comment #1)
> What happens if you repair the target folder? Right-click folder,
> Properties, Repair.

Thank you for looking.
No improvement at all. Anyway, this happens with ALL the folders.
Attached image the "find" window
Thank you, Jorg
I use a french version so we don't have the same shortcuts. I created an attachment to show the "find" window.
In the sub menu, there are 4 different types of search. I use the 3rd one, "search in the mail" (after search, search the next, and before search the adresses.

The found messages are in a local folder. They are moved to a local folder.

I don't see any other parameter I could take in account. It's been happening for months, it happens systematically, whatever I do before.
OK, so you're using "regular" "Search Messages", finding messages in a local folder and in the "Search Messages" panel (find window) you're moving the messages to another folder.

When you drag the message from the "find window" to another folder everything works fine for me, but when I use the "Move To" button to move the message, I indeed get phantom messages in the target folder.

They go away when repairing the folder. Please do not attempt to delete those messages.

This is pretty severe. I'll look into it or get someone else to look into it. Aceman, can you take a look. This stock-standard operation is causing real corruption.
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: creates empty mails year 1970 when moving mails after using advanced search → Empty mails from year 1970 created when moving mails between local folders using the "Move To" button on the "Search messages" panel
Maybe a complete duplicate of bug 1110583 or bug 1106225 (see bug 1106225 comment #23).
See Also: → 1110583, 1106225
FYI.

Problem of bug 1110583 still exists
=> Due to same cause as SeaMonkey's bug 1001071, issue of bug 1110583 was easiliy exposed as phenomenon of bug 1106225.
=> By some changes in Tb45, phenomenon of bug 1106225 was morphed to "no duped mails, many consistent phantom mails", then this bug was opened.

In other words, "problem of SeaMonkey bug 1001071 in Thunderbird" is pretty powerful tool to generate phenomenon of bug 1110583.
Thank you.
I have been deleting these phantom mails for months, more than one year, without any problem but the lost time.

When I tried the repair solution, I got several other problems. First, if the destination folder is inside another folder, I am obliged to do it 2 times, repairing the main, and then the sub folder. When it is finished, another problem : some old emails are randomly declared not read (and in a box containing 500 messages, I need to sort the mails by status, and then set them read, which is a pit.ass.
I hope someone can repair this problem, it would be a real relief for me.
Thanks
(In reply to Jorg K (GMT+1) from comment #7)
> Maybe a complete duplicate of bug 1110583 or bug 1106225 (see bug 1106225 comment #23).

SeaMonkey's bug 1001071 was pointed by bug 1106225 comment #16 on 2014-12-11, so following was already known in 2014.
 Reason why "same issue as bug 1106225 in Thunderbird" can so pretty easily generate issue of bug 1110583
 == phenomenon of bug 1106225 == phenomenon of bug 1319299(this bug)
Because root cause of problems is bug 1110583, I thought that developers intentionally didn't port change by SeaMonkey's bug 1106225 to Thunderbird. (It's not essential solution, it's merely workaround, ...)

I think duping this bug to other bug is better avoided.
I believe "port bug 1001071 to Thunderbird" is better done in this bug, because external symptom of this bug is pretty clear.
(In reply to ddegan from comment #9)
> another problem : some old emails are randomly declared not read (and in a
> box containing 500 messages, ...
Yes, "repair folder" will bring all messages marked as read by "Mark Folder Read" back to life. You can just repeat that. Also, the Quick Filter offers an "Unread" button, so you get all unread messages with one click ;-)

(In reply to WADA from comment #10)
Thanks for all the background information. I couldn't believe my eyes seeing corruption created by just one click.

I'll read through bug 1110583 (root cause), the SM work-around fix (bug 1001071, which could be ported), bug 1106225 (which is the same as this bug here).

Why don't we dupe this bug onto bug 1106225 and fix that one, one way or another?
I did a bit of reading and I'm confused now. In comment #10 you said:
  I believe "port bug 1001071 to Thunderbird" is better done in this bug, ...
If I understand this correctly, you want to port something from bug 1001071 to TB.

However, in bug 1001071 comment #14 we read that bug 1001071 *caused* bug 1106225 which is the bug we want to fix. Also, bug 1106225 comment #16 seems to confirm that, see "oops".

Anyway, apparently if comes down to a badly configured menu where messages are moved more than once creating duplicates or phantoms (if the original was already deleted).
(In reply to Jorg K (GMT+1) from comment #11)
> (In reply to WADA from comment #10)
> Why don't we dupe this bug onto bug 1106225 and fix that one, one way or another?

That bug has many referring to too many causes of symptoms/issues of that bug.
 (a) : Bad menu definition in Search Panel generates many "move messages" requests at same time by merely 1 click
  => (b) : due to (a), problem of bug 1110583 is easily produced and exposed
  => (c) : there are many causes of symptoms of bug 1110583 in many relevant components.
It looks for me that developers want to try to resolve all causes of above symptoms/phenomena/issues at once in that bug.
If not, why simple/easy "port bug 1001071 to Thunderbird" part, which was already known in 2014, is still not done in that bug?
So, never DUP.

(a) is never actual cause of problems. It merely drastically increases probability of that unwanted problems happen.
"Removing bad menu definition in Search Panel" is better proposed/landed by this bug independently from that bug.
That bug's Whiteboard like "a part of issues has been fixed by bug 1319299" is sufficient.
No longer blocks: 209501
Depends on: 209501
This appears to fix the problem. As per the previous comment, that's what SM does, so it can't be 100% wrong ;-)
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8813258 - Flags: review?(acelists)
(In reply to Jorg K (GMT+1) from comment #14)
> Hmm, in SM the command is on the button:
> https://dxr.mozilla.org/comm-central/rev/b8ec04edc0e3e7248d01607653cb0bffb132aa6f/suite/mailnews/search/SearchDialog.xul#149
> In TB it's on the popup:
> https://dxr.mozilla.org/comm-central/rev/b8ec04edc0e3e7248d01607653cb0bffb132aa6f/mail/base/content/SearchDialog.xul#189

The menupopup is type="folder" popup for folder picker.
oncommand of the type="folder" menupopup is invokerd at each folder hiearchy level.
 => we can invoke many "move NN mails from Inbox to Target" at once by merely 1 click of a button :-)
This is perhaps design/spec/implementation of type="folder" since initial of type="folder".
So, it's merely a misuse of type="folder" menupopup.
In SeaMonkey, this was already corrected by bug 1001071 at 2014/04, and oncommand/command is already moved to upper button/menu.

These are what I was taught by neil@parkwaycc.co.uk in bug 1106225 comment #16 on 2014-12-11.
So you're saying that my patch is correct, right?
(In reply to Jorg K (GMT+1) from comment #17)
> So you're saying that my patch is correct, right?

bug 1133212
OK, that should be an easy review then. It's just that Wada kept talking about bug 1001071 which seems to have caused the problem which was then fixed in bug 1133212. Let's get this fixed and landed, not only for the reporter. Creating corruption with a single click is just not on at all.
Sorry, correction of my wrong description on history.

In SeaMonkey, oncommand on menupopup of type="folder" && mode="filing" in SearchDialog.xul was introduced by Bug 1001071 at 2014/04,
and the oncommand was moved to upper button in the SearchDialog.xul by Bug 1133212 at 2015-03-05.
It was perhaps confusion with menupopup of type="folder" && mode="search" and menupopup of type="folder" && mode="filters" which are used in other elements.

Unfortunately, change in SearchDialog.xul part by Bug 1001071 was ported to Thunderbird before correction by Bug 1133212 was landed.
But, no one ported correction by Bug 1133212 to Thunderbird after fix of the Bug 1133212.
Even after we reported Bug 1106225 for Thunderbird, no one still ported correction by Bug 1133212 to Thunderbird.
It was perhaps because actual cause of problems is Bug 1110583, instead of UI definition in SearchDialog.xul.

Difference of philosophy is seen between Thunderbird developers and SeaMonkey developers.
 one     : Root cause of problems should be resolved. 
 another : Even if it's never root cause of problems, bad definition should be corrected.

(In reply to Jorg K (GMT+1) from comment #17)
> So you're saying that my patch is correct, right?

Before I post this comment, aceman pointed Bug 1133212 in this bug.
Your patch is absolutely same as correction by Bug 1133212.
We should have opened separate bug for "port Bug 1133212 to Thunderbird", instead of requesting it in Bug 1106225.
Summary: Empty mails from year 1970 created when moving mails between local folders using the "Move To" button on the "Search messages" panel → Empty mails from year 1970 created when moving mails between local folders using the "Move To" button on the "Search messages" panel (porting SeaMonkey Bug 1133212 to Thunderbird is needed)
Comment on attachment 8813258 [details] [diff] [review]
Solution à la SM (v1)

Review of attachment 8813258 [details] [diff] [review]:
-----------------------------------------------------------------

OK this works for me.

But there was bug 878805 to consolidate the occurences and always have oncommand on the menupopup.
In the same SearchDialog.xul file, you'll see another menupopup with type="folder" (the picker of the folder on which to run the search). If you instrument it, you'll its function updateSearchFolderPicker() is run multiple times (once for each level of the folder picker you expand). So the root cause of this problem for other pickers is not solved yet, they just do exhibit a problem if run multiple times.

So if you want, we can land this patch on trunk and branches and I make a new bug for fixing the problem properly (not cloning the oncommand to all levels).
Attachment #8813258 - Flags: review?(acelists) → review+
https://hg.mozilla.org/comm-central/rev/a26cd7d8d5343484461ecac950baffa9da9936ac

Let's go with the "improper" fix for now ;-)
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 53.0
Attachment #8813258 - Flags: approval-comm-beta?
Attachment #8813258 - Flags: approval-comm-aurora+
(In reply to ddegan from comment #9)
> I hope someone can repair this problem, it would be a real relief for me.
You'll get the fix in TB 51 beta 2 (if we ever manage to produce beta 1 first which is currently stalled by bug 1317863), TB 52 Earlybird after I uplift on the weekend or TB 52 ESR in March 2017.
Thanks for finding the solution so fast Jorg.
The thanks goes to Wada. Without him I would have had a bad day debugging this.

Interesting reading in bug 878805 from bug 878805 comment #35 downwards. So I'm eagerly awaiting Aceman's bug to fix things further.
(In reply to :aceman from comment #21)
> But there was bug 878805 to consolidate the occurences and always have oncommand on the menupopup.
> In the same SearchDialog.xul file, you'll see another menupopup with type="folder" (the picker of the folder on which to run the search). 

In change by Bug 1001071, 3 kinds of type=folder" are seen.
  menupopup type="folder" mode="filing"
  menupopup type="folder" mode="filters"
  menupopup type="folder" mode="search"
Parent element is different in these 3 cases.
I don't know spec of type="folder", each mode attribute, difference in behavior by parent element.

As we learned by Bug 1133212 and this bug, in folder picker under <menupopup type="folder" mode="xxx" oncommand="A_Function()">, click event at a folder seems to invoke specified A_Function() at every directory hierarchy level.
I think that we are better to check behavior of <menupopup type="folder"> with each mode attribute, because Bug 878805 moved oncommand="xxx()" in parent element to child <menupopup type="folder" mode="???" oncommand="xxx()">.

Bug 878805 was fixed on 2014-01-27.
Was 'menupopup type="folder" mode="filing" oncommand="MoveMessageInSearch(event.target)' ported and removed like next?
  Bug 878805 in Thunderbird(2014-01) -> Bug 1001071 in SeaMonkey(2014-04)
  -> oncommand was moved back to parent by Bug 1133212 in SeaMonkey(2015-03)
  -> oncommand was moved back to parent by this bug in Thunderbird(2016-11-22)
Comment on attachment 8813258 [details] [diff] [review]
Solution à la SM (v1)

[Approval Request Comment]
Regression caused by (bug #): Bug 878805 (shifted "oncommand")
User impact if declined: Terrible. Phantom messages (corruption!!)
Testing completed (on c-c, etc.): Manual.
Risk to taking this patch (and alternatives if risky):
Very low, only moving "oncommand" from popup back to its parent.
Attachment #8813258 - Flags: approval-comm-esr45?
Attachment #8813258 - Flags: approval-comm-beta?
Attachment #8813258 - Flags: approval-comm-beta+
Comment on attachment 8813258 [details] [diff] [review]
Solution à la SM (v1)

https://hg.mozilla.org/releases/comm-esr45/rev/ca0fdc5651ed
Attachment #8813258 - Flags: approval-comm-esr45? → approval-comm-esr45+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: