Closed Bug 1292569 Opened 3 years ago Closed 3 years ago

Port |Bug 1199296 - Don't allow legacy generator yield in method definitions| to Thunderbird

Categories

(Thunderbird :: General, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 51.0

People

(Reporter: jorgk, Assigned: aceman)

References

()

Details

Attachments

(4 files, 2 obsolete files)

First seen on Daily build 2016-08-05:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=43547
https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=43543

Massive test failures:
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_emptyTrash_dbViewWrapper.js | xpcshell return code: 0
SyntaxError: non-generator method definitions may not contain yield at ../../../../mailnews/resources/messageModifier.js:133
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_imapFolder.js | xpcshell return code: 0
SyntaxError: non-generator method definitions may not contain yield at ../../../../mailnews/resources/messageModifier.js:133
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_virtualFolderCustomTerm.js | xpcshell return code: 0
SyntaxError: non-generator method definitions may not contain yield at ../../../../mailnews/resources/messageModifier.js:133
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_logic.js | xpcshell return code: 0
SyntaxError: non-generator method definitions may not contain yield at ../../../../mailnews/resources/messageModifier.js:133
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_virtualFolder.js | xpcshell return code: 0
SyntaxError: non-generator method definitions may not contain yield at ../../../../mailnews/resources/messageModifier.js:133 

And many more.
Bug 1199296
So do we need to do conversions like the one in the patch?
-  get nodes() {
+  *nodes() {
And then the callers:
-    for (let node of SocialMarks.nodes) {
+    for (let node of SocialMarks.nodes()) {
Yes, we have to get rid of the obsolete nonstandard syntax . I wonder if there's a good way to find all instances (obviously the test failures help).
Probably just grep for yield and look at the containing function.

I try to look at it.
I converted some occurrences and some tests improved:
https://hg.mozilla.org/try-comm-central/rev/8aca6940ad476ff3d79b788301e4631569dbacc5

But I'm still stuck at this problem:
 EXCEPTION: folderBatch.messages is undefined
    at: messageInjection.js line 676
       add_sets_to_folders messageInjection.js:676 11
       make_new_sets_in_folders messageInjection.js:560 3
       test_folder_names_in_recent_view_mode test-folder-names-in-recent-mode.js:52 3
       Runner.prototype.wrapper frame.js:585 9
       Runner.prototype._runTestModule frame.js:655 9
       Runner.prototype.runTestModule frame.js:701 3
       Runner.prototype.runTestDirectory frame.js:525 7
       runTestDirectory frame.js:707 3
       Bridge.prototype._execFunction server.js:179 10
       Bridge.prototype.execFunction server.js:183 16
       Session.prototype.receive server.js:283 3
       AsyncRead.prototype.onDataAvailable server.js:88 3
The _looperator? It is function* in my patch
https://hg.mozilla.org/try-comm-central/rev/23ca1339c301ad0b87706e6b429a2e1307d7f84e
This one could be better:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=0c4b34d61665bb2f8bf9a55269762488be18d3d7

Looks like we need to use iterator.next().value, not just iterator.next(). I'll look at some more cases later in the day.
Thanks for looking into it, this seems like a real pain. I prefer bustage that you can fix with sed ;-)
Haven't we got any JS experts who could help/advise here?
Summary: TEST-UNEXPECTED-FAIL | (many occurences) SyntaxError: non-generator method definitions may not contain yield at → Port |Bug 1199296 - Don't allow legacy generator yield in method definitions| to Thunderbird
Mozmill on latest try is looking promising, Xpcshell also better than before. Looks like we're on the home stretch ;-)
Some of the changes are needed in the core code. Even if we fix the tests, some surprises may still lurk while running TB normally (tests do not cover all code paths). 'Thanks' to the JS non-typed behaviour, if I change a function to proper generator, I have to hunt for all the callers manually (no compiler finds the now non-type-matching callers automatically). So there may be remnants, that may be found only by running nightlies in production.
I CC our QA guys to watch for these problems and alert us in follow-up bugs. Thanks.
Severity: normal → critical
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Attached patch fix for mozmill (obsolete) — Splinter Review
This fixes the mozmill tests:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=cb176244084fc11fb237c15a14cf9eacddf92e1c

xpcshell tests are a different beast as the tests themselves are often made as generators. And that seems to create more problems in asyncTestUtils.js if they are converted to function* . I need to find out what the conversion pattern there is.
Flags: needinfo?(rkent)
Attachment #8778679 - Flags: review?(mkmelin+mozilla)
The patch does not cover Seamonkey as I can't test it easily.
Comment on attachment 8778679 [details] [diff] [review]
fix for mozmill

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

::: mail/base/modules/MailUtils.js
@@ +381,3 @@
>          timer.cancel();
>          if (aCallback)
> +          aCallback(!thrown);

Not sure this is correct. It seems to change the logic slightly. Also I'd just do the timer cancelling and calling of callback directly like before, not keeping variables (which seem to change the logic)

::: mail/test/mozmill/shared-modules/test-folder-display-helpers.js
@@ +890,5 @@
>      aController = mc;
>    // SyntheticMessageSet special-case
>    if (typeof(aViewIndex) != "number") {
> +    let msgHdrIter = aViewIndex.msgHdrs();
> +    let msgHdr = msgHdrIter.next().value;

could just be

 let msgHdr = aViewIndex.msgHdrs().next().value;

::: mailnews/db/gloda/modules/datastore.js
@@ +3168,1 @@
>                                                                aValues) {

nit: align aValues one more space

::: mailnews/db/gloda/modules/explattr.js
@@ +138,1 @@
>                                             aCallbackHandle) {

align once space

::: mailnews/db/gloda/modules/index_msg.js
@@ +3230,1 @@
>                                                       aCallbackHandle) {

align one more space

::: mailnews/test/resources/messageModifier.js
@@ +123,5 @@
>    /**
>     * @return a JS iterator of the message headers for all messages inserted into
>     *     a folder.
>     */
> +  *msgHdrs() {

Is this correct? Would have expected 

msgHdrs: function*() {

@@ +167,5 @@
>    /**
>     * @return a generator that yields [nsIMsgFolder, nsIMutableArray of the
>     *     messages from the set in that folder].
>     */
> +  *foldersWithXpcomHdrArrays() {

same thing here??

foldersWithXpcomHdrArrays: function*() {
Comment on attachment 8778679 [details] [diff] [review]
fix for mozmill

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

::: mail/test/mozmill/downloads/test-about-downloads.js
@@ +51,5 @@
>      this.removedItems.push(aDownload.target.path);
>      this.items.delete(aDownload);
>    },
>  
> +  *waitForFinish() {

Magnus complained about something similar below. So should this be waitForFinish: function* () {
(In reply to Magnus Melin from comment #15)
> > +  *msgHdrs() {
> 
> Is this correct? Would have expected 
> 
> msgHdrs: function*() {

It means the same thing, just written as an ES6 method. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Method_definitions
(In reply to Magnus Melin from comment #15)
> ::: mail/base/modules/MailUtils.js
> @@ +381,3 @@
> >          timer.cancel();
> >          if (aCallback)
> > +          aCallback(!thrown);
> 
> Not sure this is correct. It seems to change the logic slightly. Also I'd
> just do the timer cancelling and calling of callback directly like before,
> not keeping variables (which seem to change the logic)

It didn't work without this change. First it seems generators do not throw the exception StopIteration that we were expecting here. They return .done = true and .value = undefined. And StopIteration is deprecated and will be removed too.

But the change should not change the logic, just use the proper properties on generator to get the same log.
So in which case do you see a logic change?

> ::: mailnews/test/resources/messageModifier.js
> @@ +123,5 @@
> >    /**
> >     * @return a JS iterator of the message headers for all messages inserted into
> >     *     a folder.
> >     */
> > +  *msgHdrs() {
> 
> Is this correct? Would have expected 
> 
> msgHdrs: function*() {

Somehow I got the idea that it is the same.
(In reply to :aceman from comment #18)
> But the change should not change the logic, just use the proper properties
> on generator to get the same log.
> So in which case do you see a logic change?

Thx for the explanation, there's no change then. I'd still not mix in the variables but just

try {
  if (worker.next().done) {
    timer.cancel();
    if (aCallback)
      aCallback(true);
  }
} catch (ex) {
  timer.cancel();
  if (aCallback)
    aCallback(false);
}

> > > +  *msgHdrs() {
> > 
> > Is this correct? Would have expected 
> > 
> > msgHdrs: function*() {
> 
> Somehow I got the idea that it is the same.

Seems they are (per aleth's link). The non-shorthand seems more readable to me though.
(In reply to Magnus Melin from comment #19)
> Thx for the explanation, there's no change then. I'd still not mix in the
> variables but just
> 
> try {
>   if (worker.next().done) {
>     timer.cancel();
>     if (aCallback)
>       aCallback(true);
>   }
> } catch (ex) {
>   timer.cancel();
>   if (aCallback)
>     aCallback(false);
> }

OK, I wanted to avoid this code duplication but yes, I can do this so it is obvious it should be the same.
 
> > > > +  *msgHdrs() {
> > > 
> > > Is this correct? Would have expected 
> > > 
> > > msgHdrs: function*() {
> > 
> > Somehow I got the idea that it is the same.
> 
> Seems they are (per aleth's link). The non-shorthand seems more readable to
> me though.

To me too :)
Attachment #8778679 - Attachment is obsolete: true
Attachment #8778679 - Flags: review?(mkmelin+mozilla)
Attachment #8778702 - Flags: review?(mkmelin+mozilla)
So is this an r+ then with the nits fixed that we can land?
Any test bustage is pretty fatal since new problems are creeping in unnoticed, very recently for example:
Bug 1246447 comment #15.
Comment on attachment 8778702 [details] [diff] [review]
fix for mozmill v1.1 [checked in in comment 24]

I checked that all the complaints are addressed, so we should land this.
Not sure that Magnus is still online.
Attachment #8778702 - Flags: review+
Mozmill part landed:
https://hg.mozilla.org/comm-central/rev/7b9fbef870b2
Keywords: leave-open
Attachment #8778702 - Attachment description: fix for mozmill v1.1 → fix for mozmill v1.1 [checked in in comment 24]
Attachment #8778702 - Flags: review?(mkmelin+mozilla)
(In reply to :aceman from comment #13)
> xpcshell tests are a different beast as the tests themselves are often made
> as generators. And that seems to create more problems in asyncTestUtils.js
> if they are converted to function* . I need to find out what the conversion
> pattern there is.
I just picked one test as random: 
mailnews/imap/test/unit/test_saveImapDraft.js

In there:
function createDraftsFolder() {
  IMAPPump.incomingServer.rootFolder.createSubfolder("Drafts", null);
  yield false;

So adding the * there doesn't help?

Looks like the failed tests
https://treeherder.mozilla.org/#/jobs?repo=comm-central&selectedJob=43628
are all the ones which include asyncTestUtils.js:
https://dxr.mozilla.org/comm-central/search?q=asyncTestUtils.js&redirect=false

I've noticed that function _async_test_runner in asyncTestUtils.js also has a few yield statements. Hmm.
[Just a few ignorant observations ;-( ]
It does not help by itself.


The comment at async_run() in asyncTestUtils.js say that tests can be functions the return OR yield a true/false. But converting a yielding function to function* changes the syntax slightly so I need to find what to change in asyncTestUtils.js .

The main problem seems to be here:
"Unhappiness at: ../../../../mailnews/resources/asyncTestUtils.js:156"
 0:01.41 PROCESS_OUTPUT: Thread-1 (pid:11255) "Because: TypeError: curGenerator.send is not a function"

Maybe we can't call send on a real generator function.
(In reply to :aceman from comment #26)
> The comment at async_run() in asyncTestUtils.js say that tests can be
> functions the return OR yield a true/false. But converting a yielding
> function to function* changes the syntax slightly so I need to find what to
> change in asyncTestUtils.js .

The more modern/simpler way of doing async tests is https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests#Task-based_asynchronous_tests but it's probably quicker to just update asyncTestUtils.js.

> The main problem seems to be here:
> "Unhappiness at: ../../../../mailnews/resources/asyncTestUtils.js:156"
>  0:01.41 PROCESS_OUTPUT: Thread-1 (pid:11255) "Because: TypeError:
> curGenerator.send is not a function"
> 
> Maybe we can't call send on a real generator function.

Yes, .send is some ancient nonstandard syntax, I have no idea what it did... maybe there is still some old documentation around somewhere.
I think rkent wanted to get rid of asyncTestUtils in the long run, so this could be the chance, if it isn't too tedious.

Yes, I also can't find documentation what function.send() does. There is only .bind(), .call(), .apply() methods described for function.prototype.
I think I'm onto something here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6d2a7a19056d1bb74d7bebb84f758233e20b7b45

Locally all the tests passed for me in /imap /compose /news /base /mime.
(In reply to :aceman from comment #12)
> So there may be remnants, that may be found only by running
> nightlies in production.
Due to Windows bustage (bug 1282256) it was impossible to run Daily TB 51a1 in production until today. I've just installed it and will look out for any problems.

Here comes the first one: Gloda. I'm using Gloda only one single folder, the folder where I move my BMO mail. After starting TB 51 and looking in the activity manager, this hangs with: Indexing messages, Determining which messages to index.
Could this be this error I got?

Mon Aug 08 2016 14:04:08
Error: TypeError: this.activeIterator.close is not a function
Source file: resource://app/modules/gloda/indexer.js
Line: 897
I think so. I was going to rebuild tonight, but Richard was quicker ;-)
(In reply to :aceman from comment #14)
> The patch does not cover Seamonkey as I can't test it easily.
Is there a dummies guide? I know almost nothing about generator yields.
(In reply to Richard Marti (:Paenglab) from comment #31)
> Mon Aug 08 2016 14:04:08
> Error: TypeError: this.activeIterator.close is not a function
> Source file: resource://app/modules/gloda/indexer.js
> Line: 897

I see it in the xpcshell tests, I'll try to fix it.

(In reply to Philip Chee from comment #33)
> (In reply to :aceman from comment #14)
> > The patch does not cover Seamonkey as I can't test it easily.
> Is there a dummies guide? I know almost nothing about generator yields.

Maybe start here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function*
Also see the type of changes my patch does (and also the other one for xpcshell on try server).
Depends on: 1293241
Attached patch fix for xpcshell (obsolete) — Splinter Review
This works for me locally and makes ALL xpcshell tests pass (a feat for my setup:))

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e8fbc93711ebee9b3502db9e3d5d2c13c4ac2524

Apart from just adding * everywhere (where yield is called), the real interesting change is in asyncTesUtils.js (end of patch).
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #8779040 - Flags: review?(rkent)
Attachment #8779040 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8778702 [details] [diff] [review]
fix for mozmill v1.1 [checked in in comment 24]

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

::: mail/test/mozmill/shared-modules/test-folder-display-helpers.js
@@ -891,5 @@
>    // SyntheticMessageSet special-case
>    if (typeof(aViewIndex) != "number") {
> -    let msgHdrIter = aViewIndex.msgHdrs;
> -    let msgHdr = msgHdrIter.next();
> -    msgHdrIter.close();

As discussed on IRC, according to https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Generator this needs to be .return()
Flags: needinfo?(rkent)
Keywords: leave-open
Slight fix to previous version, .close() -> .return().

Try run with this:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c007c31ebcd2c52db528655c24488837e168e2dc
Attachment #8779040 - Attachment is obsolete: true
Attachment #8779040 - Flags: review?(rkent)
Attachment #8779040 - Flags: review?(mkmelin+mozilla)
Attachment #8779085 - Flags: review?(rkent)
Attachment #8779085 - Flags: review?(mkmelin+mozilla)
Attachment #8779085 - Flags: feedback?(bugmail)
Comment on attachment 8779085 [details] [diff] [review]
fix for xpcshell v1.1 [checked in in comment 39]

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

The asterisk store called; they're running out of asterisks! ;)
Attachment #8779085 - Flags: feedback?(bugmail) → feedback+
Xpcshell part landed:
https://hg.mozilla.org/comm-central/rev/378b9a66d495
I'm leaving this open for further adjustments for now.
We need to wait for the asterisk store to produce more.

Maybe Kent or Magnus want to do a post-landing review, especially here:
https://hg.mozilla.org/comm-central/rev/378b9a66d495#l105.13 and here:
https://hg.mozilla.org/comm-central/rev/378b9a66d495#l20.21 and here:
https://hg.mozilla.org/comm-central/rev/378b9a66d495#l7.12 (.close()/.return() restored from first patch)
https://hg.mozilla.org/comm-central/rev/7b9fbef870b2#l12.12 (mistakenly removed here).

Note: Patch converted mailnews/imap/test/unit/test_subfolderLocation.js from CRLF line endings to LF endings without any further changes. (Which also made the patch fail to apply on Windows. I managed.)
Keywords: leave-open
Attachment #8779085 - Attachment description: fix for xpcshell v1.1 → fix for xpcshell v1.1 [checked in in comment 39]
Attachment #8779085 - Flags: review+
Comment on attachment 8779085 [details] [diff] [review]
fix for xpcshell v1.1 [checked in in comment 39]

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

::: mailnews/test/resources/asyncTestUtils.js
@@ +154,5 @@
>      curGenerator = asyncGeneratorStack[asyncGeneratorStack.length-1][0];
>      try {
> +      let nextItem = true;
> +      while (nextItem) {
> +        nextItem = curGenerator.next(asyncGeneratorSendValue);

I'd avoid mixing variable types like this. nextItem will never be anything falsy anyway, so you can just have this for clearer code

while(true) {
  let nextItem = .....

@@ +155,5 @@
>      try {
> +      let nextItem = true;
> +      while (nextItem) {
> +        nextItem = curGenerator.next(asyncGeneratorSendValue);
> +        if (!nextItem.value || nextItem.done)

We shouldn't continue just because the return value is falsy?
Attachment #8779085 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 8779085 [details] [diff] [review]
fix for xpcshell v1.1 [checked in in comment 39]

Sorry for the quick landing and "post-landing" review. We can land a tiny follow up patch.
Attachment #8779085 - Flags: review?(rkent)
(In reply to Magnus Melin from comment #40)
> ::: mailnews/test/resources/asyncTestUtils.js
> @@ +154,5 @@
> >      curGenerator = asyncGeneratorStack[asyncGeneratorStack.length-1][0];
> >      try {
> > +      let nextItem = true;
> > +      while (nextItem) {
> > +        nextItem = curGenerator.next(asyncGeneratorSendValue);
> 
> I'd avoid mixing variable types like this. nextItem will never be anything
> falsy anyway, so you can just have this for clearer code

OK.

> while(true) {
>   let nextItem = .....

I need nextItem outside of the loop, but that is trivial.

 
> @@ +155,5 @@
> >      try {
> > +      let nextItem = true;
> > +      while (nextItem) {
> > +        nextItem = curGenerator.next(asyncGeneratorSendValue);
> > +        if (!nextItem.value || nextItem.done)
> 
> We shouldn't continue just because the return value is falsy?

I got the impression that the tests always yield true or false. On false, we should stop. Also compare to the original code. The loop stopped when .send() returned falsy, which is now replaced by .next().value. The .next().done is a replacement for the StopIteration exception. So I'd say I didn't want to change behaviour.
But I see a slight difference. Previously if it has thrown, it didn't set asyncGeneratorSendValue=undefined. I can fix that.
Blocks: 1123122
(In reply to :aceman from comment #42)
> I got the impression that the tests always yield true or false. On false, we
> should stop. Also compare to the original code. The loop stopped when
> .send() returned falsy, which is now replaced by .next().value. The
> .next().done is a replacement for the StopIteration exception. So I'd say I
> didn't want to change behaviour.

Yes, the contract is defined here:
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/MailNews/AsyncTestUtils_Extended_Framework#The_Asynchronous_Function_Contract

It made some sense in a pre-Promise world, but as :jcranmer has pointed out, it would have been better to come up with the Task.jsm flow instead!
Attachment #8779450 - Flags: review?(mkmelin+mozilla)
Attachment #8779450 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 8779450 [details] [diff] [review]
tweak for xpcshell [checked in in comment 53]

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

::: mailnews/test/resources/asyncTestUtils.js
@@ +163,5 @@
> +
> +      if (nextItem.done) {
> +        asyncGeneratorStack.pop();
> +      } else {
> +        asyncGeneratorSendValue = undefined;

Doesn't that always need to be reset? Why move it in here?
Attachment #8779450 - Flags: review+ → review?(mkmelin+mozilla)
Sorry, my changes overwrote Magnus' r+. Can you add it again? If I do it, it will look as if I gave the r+.
Attachment #8779459 - Flags: review?(clokep) → review+
Attachment #8779459 - Attachment description: Remove legacy generators from im/ → Remove legacy generators from im/ [checked in in comment 48]
Comment on attachment 8779450 [details] [diff] [review]
tweak for xpcshell [checked in in comment 53]

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

Adding back the r+. I'll leave the question for aceman ;)
Attachment #8779450 - Flags: review?(mkmelin+mozilla) → review+
(In reply to Jorg K (GMT+2, PTO during summer) from comment #46)
> ::: mailnews/test/resources/asyncTestUtils.js
> @@ +163,5 @@
> > +
> > +      if (nextItem.done) {
> > +        asyncGeneratorStack.pop();
> > +      } else {
> > +        asyncGeneratorSendValue = undefined;
> 
> Doesn't that always need to be reset? Why move it in here?

As said, in the original code, if the .send() threw the StopIteration exception and jumped to the catch() block, it didn't set this variable. So I try to preserve that exactly. But as tests pass either way, it may not be important.
(In reply to :aceman from comment #50)
> As said, in the original code, if the .send() threw the StopIteration
> exception and jumped to the catch() block, it didn't set this variable. So I
> try to preserve that exactly. But as tests pass either way, it may not be
> important.

Preserving it exactly makes sense for this bug. You can get rid of asyncTestUtils altogether and replace it with Tasks in another bug ;)
I haven't fully understood that that asyncGeneratorSendValue variable does since it's not looked at any more after being set. Perhaps it persists. But I'd sleep better the way you had it and the way is was landed and tested. If I had to decide, I'd change the loop and nothing else ;-)
Are you planning on another try or did you run some tests locally?
https://hg.mozilla.org/comm-central/rev/cf69eee353d3126887db17c3266d4bfa8a961d63

I would now close this bug unless there are any known open issues.
Attachment #8779450 - Attachment description: tweak for xpcshell → tweak for xpcshell [checked in in comment 53]
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 51.0
Depends on: 1294960
You need to log in before you can comment on or make changes to this bug.