Closed Bug 1237602 Opened 5 years ago Closed 5 years ago

Port Bug 1220564 - Remove legacy array/generator comprehension for comm-central

Categories

(Thunderbird :: General, defect)

defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 46.0

People

(Reporter: aleth, Assigned: aryx)

References

Details

Attachments

(6 files, 7 obsolete files)

22.00 KB, patch
aceman
: review+
Details | Diff | Splinter Review
23.28 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
17.14 KB, patch
Fallen
: review+
Details | Diff | Splinter Review
20.15 KB, patch
aleth
: review+
Details | Diff | Splinter Review
5.88 KB, patch
philip.chee
: review+
Details | Diff | Splinter Review
1.17 KB, patch
florian
: review+
Details | Diff | Splinter Review
Used in various places (it is of course breaking tests)
Blocks: 1220564
Severity: normal → blocker
It seems today Daily is completely broken by bug 1220564. Built with m-c changeset b384917df4e9 all is working.
Attached patch 1237602-1.diff (obsolete) β€” β€” Splinter Review
I've done this with some regex find/replace work, so I might've missed some (especially if they span more than one line), but it should be enough to get everything going again.
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #8705356 - Flags: review?(philipp)
Comment on attachment 8705356 [details] [diff] [review]
1237602-1.diff

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

r=philipp but also untested since my battery is about to die. One comment:

::: mail/test/mozmill/runtest.py
@@ +369,5 @@
>          if isinstance(self.options.test, basestring):
>              test_paths = [self.options.test]
>          else:
>              test_paths = self.options.test
> +        TEST_NAME = ', '.join([for t in test_paths os.path.basename(t)])

This is actually python, so I think it should stay as is?
Attachment #8705356 - Flags: review?(philipp) → review+
Comment on attachment 8705356 [details] [diff] [review]
1237602-1.diff

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

::: chat/protocols/twitter/twitter.js
@@ +128,5 @@
>    this._init(aAccount);
>    this._ensureParticipantExists(aAccount.name);
>    // We need the screen names for the IDs in _friends, but _userInfo is
>    // indexed by name, so we build an ID -> name map.
> +  let names = new Map([for ([name, userInfo userInfo.id_str, name]] of aAccount._userInfo));

This looks odd?

::: mailnews/mime/jsmime/test/test_mime_tree.js
@@ +83,5 @@
>   *                 of the file from [line start, line end) [1-based lines]
>   */
>  function make_body_test(test, file, opts, partspec) {
>    var results = Promise.all([
> +    Promise.all([for (p of partspec) p[0], read_file(file, p[1], p[2])])]);

Also odd, worth cross-checking.

Promise.all(for (p of partspec) [p[0], read_file(file, p[1], p[2])])]);

sounds more like it? I'm guessing myself on this one, tb;dr. (b being braced)
> [for (foo in bar) foo]

is a syntax error also, and I found 41 of those in c-c, so that's still broken.
Given bug 1220564 comment 2, it would be better to get rid of comprehensions altogether, especially in dubious cases.
Attached patch 1237602-1.1.diff (obsolete) β€” β€” Splinter Review
Fixes some mistakes in the first patch.
Attachment #8705356 - Attachment is obsolete: true
Attached patch 1237602-2.diff (obsolete) β€” β€” Splinter Review
More stuff I've found since the first patch.
I've had enough of this mess. There's a patch on try now: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2a9893e7d1ce . If it doesn't pass I'm unassigning myself from this bug.
Assignee: geoff → nobody
Status: ASSIGNED → NEW
Geoff, thanks for the start. I hope we can finish it from here now.

I can look at this today. But if anybody is interested too, we can split the work.
OS: Unspecified → All
Hardware: Unspecified → All
Some additional info on running the script from the original bug from Shu: "i'd run the shell with |path/to/shell -e 'jsversion(185)' find-comps.js|. also, that script won't help you with embedded JS in XUL, HTML files, or ones that require preprocessing with #-directives"
> I can look at this today. But if anybody is interested too, we can split the work.
Generating the file lists at the moment.
Duplicate of this bug: 1238159
Duplicate of this bug: 1238138
Duplicate of this bug: 1238400
Duplicate of this bug: 1238071
Attached patch chatim, v1 (obsolete) β€” β€” Splinter Review
Assignee: nobody → aryx.bugmail
Attachment #8705504 - Attachment is obsolete: true
Attachment #8705505 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8706335 - Flags: review?(aleth)
Attached patch mail, v1 β€” β€” Splinter Review
Attachment #8706336 - Flags: review?(acelists)
Attached patch mailnews, v1 β€” β€” Splinter Review
Attachment #8706337 - Flags: review?(mkmelin+mozilla)
Attached patch suite, v1 (obsolete) β€” β€” Splinter Review
Attachment #8706339 - Flags: review?(philip.chee)
Comment on attachment 8706335 [details] [diff] [review]
chatim, v1

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

::: chat/components/src/imContacts.js
@@ +332,5 @@
>    get name() { return "__others__"; },
>    set name(aNewName) { throw Cr.NS_ERROR_NOT_AVAILABLE; },
>    getContacts: function(aContactCount) {
> +    let contacts = [];
> +    for each (contact in this._contacts) {

Let's not add any new deprecated for each loops. Arai has already removed them from some parts of c-c and has WIP patches for the rest.

Here you could use for...of or
let contacts = [...this._contacts];

@@ +1323,5 @@
>    getContactById: aId => ContactsById[aId],
>    // Get an array of all existing contacts.
>    getContacts: function(aContactCount) {
> +    let contacts = Object.keys(ContactsById)
> +                         .filter(id => !ContactsById[id]._empty);

This will produce a subset of keys rather than values. How about
let contacts = Object.values(ContactsById).filter(contact => !contact._empty));

::: chat/modules/NormalizedMap.jsm
@@ +23,5 @@
>      throw "NormalizedMap must have a normalize function!";
>    this._normalize = aNormalize;
>    // Create the wrapped Map; use the provided iterable after normalizing the
>    // keys.
> +  let entries = aIterable.map(([key, val]) => [aNormalize(key), val]);

You can't assume aIterable is an Array, so you need
let entries = [...aIterable].map(...
or a for...of loop.

::: chat/protocols/twitter/twitter.js
@@ +516,5 @@
>      hmac.init(hmac.SHA1,
>                keyFactory.keyFromString(Ci.nsIKeyObject.HMAC, signatureKey));
>      // No UTF-8 encoding, special chars are already escaped.
> +    let bytes = [];
> +    for each (let b in signatureBase) {

let bytes = [...signatureBase].map(b => b.charCodeAt());

As far as I can tell, using for...each here (on a string!) was buggy before and after this change.
Attachment #8706335 - Flags: review?(aleth) → review-
Comment on attachment 8706343 [details] [diff] [review]
calendar, v1

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

The changes look sane to me, I have no idea why this should be causing crashes in debug builds. I guess you could try commenting out some code from this patch on a local debug build (leaving arrays empty) and trying to figure out what js code causes this to crash.

::: calendar/base/src/calIcsParser.js
@@ +54,5 @@
> +                if (prop.propertyName != "VERSION" && prop.propertyName != "PRODID") {
> +                    props.push(prop);
> +                }
> +            }
> +            this.mProperties = this.mProperties.concat(props);

No need to use separate arrays, you can just use this.mProperties.push(prop);
Attachment #8706343 - Flags: review?(philipp) → review+
Comment on attachment 8706343 [details] [diff] [review]
calendar, v1

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

::: calendar/providers/gdata/modules/gdataUtils.jsm
@@ +374,5 @@
>          let attendees = aItem.getAttendees({});
> +        let attendeeData = [];
> +        for each (let a in attendees) {
> +            attendeeData.push(createAttendee(a));
> +        }

You could also write this as:

let attendeeData = attendees.map(createAttendee);
or even let attendeeData = aItem.getAttendees({}).map(createAttendee);
(In reply to Philipp Kewisch [:Fallen] from comment #23)
> The changes look sane to me, I have no idea why this should be causing
> crashes in debug builds. I guess you could try commenting out some code from
> this patch on a local debug build (leaving arrays empty) and trying to
> figure out what js code causes this to crash.

And it's always possible there is new unrelated bustage... ;)
Comment on attachment 8706337 [details] [diff] [review]
mailnews, v1

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

Looks good with a couple of nits. r=mkmelin

::: mailnews/db/gloda/modules/index_msg.js
@@ +1881,5 @@
>      this._log.info("Queueing all accounts for indexing.");
>  
>      GlodaDatastore._beginTransaction();
> +    for (let account in fixIterator(MailServices.accounts.accounts,
> +                                Ci.nsIMsgAccount)) {

odd indention  here. align MailServices and Ci perhaps?

::: mailnews/test/resources/messageInjection.js
@@ +690,5 @@
>  
>        let folder = folderBatch.folder;
>        folder.gettingNewMessages = true;
> +      let messageStrings = folderBatch.messages.map(message =>
> +                            message.synMsg.toMboxString());

nit: looks like you should indent one more space
Attachment #8706337 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 8706336 [details] [diff] [review]
mail, v1

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

Thanks for working on this!
Looks good to me with small nits.

Is this error message expected? It seems new to me.
TEST-START | /mail/test/mozmill/cloudfile/test-cloudfile-add-account-dialog.js | test_lone_provider_auto_selected
JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 150: TypeError: this._settings.contentDocument.body is undefined
JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js, line 150: TypeError: this._settings.contentDocument.body is undefined

::: mail/base/content/multimessageview.js
@@ +504,5 @@
>    summarize: function(aMessages) {
>      let messageList = document.getElementById("message_list");
>  
>      // Remove all ignored messages from summarization.
> +    let summarizedMessages = Array.from(aMessages).filter(msg => !msg.isKilled);

This seems to me this could be slower/more memory hungry than the original. You first push all items into new array and then filter them. The original only pushed the matching items.

::: mail/components/devtools/modules/XULRootActor.js
@@ +171,5 @@
>  
>      this._mustNotify = true;
>      this._checkListening();
>  
> +    return Promise.resolve(this._actorByBrowser.map(([_, actor]) => actor));

Does this work? It seems to me this._actorByBrowser is a Map (which does not have .map()), not an array.

::: mail/components/test/unit/test_about_support.js
@@ +152,5 @@
>  function test_get_account_details() {
>    let accountDetails = AboutSupport.getAccountDetails();
>    let accountDetailsText = uneval(accountDetails);
>    // The list of accounts we are looking for
> +  let accountsToSee = Object.keys(gAccountMap);

Looks like gAccountMap should be a Map ?
Attachment #8706336 - Flags: review?(acelists) → review+
Attached patch chatim, v2 β€” β€” Splinter Review
(In reply to aleth [:aleth] from comment #22)
> @@ +1323,5 @@
> >    getContactById: aId => ContactsById[aId],
> >    // Get an array of all existing contacts.
> >    getContacts: function(aContactCount) {
> > +    let contacts = Object.keys(ContactsById)
> > +                         .filter(id => !ContactsById[id]._empty);
> 
> This will produce a subset of keys rather than values. How about
> let contacts = Object.values(ContactsById).filter(contact =>
> !contact._empty));
Unfortunately, Object.values is only enabled on Nightly. Added |.map(id => ContactsById[id])| to the current code.
Attachment #8706335 - Attachment is obsolete: true
Attachment #8706632 - Flags: review?(aleth)
Comment on attachment 8706632 [details] [diff] [review]
chatim, v2

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

Thanks!
Attachment #8706632 - Flags: review?(aleth) → review+
(In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #29)
> Unfortunately, Object.values is only enabled on Nightly.

oh, good catch!
(In reply to :aceman from comment #28)
> Is this error message expected? It seems new to me.
> TEST-START |
> /mail/test/mozmill/cloudfile/test-cloudfile-add-account-dialog.js |
> test_lone_provider_auto_selected
> JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js,
> line 150: TypeError: this._settings.contentDocument.body is undefined
> JavaScript error: chrome://messenger/content/cloudfile/addAccountDialog.js,
> line 150: TypeError: this._settings.contentDocument.body is undefined
Not present in the Mozmill Try run: https://treeherder.mozilla.org/logviewer.html#?job_id=14598&repo=try-comm-central

> ::: mail/base/content/multimessageview.js
> @@ +504,5 @@
> >    summarize: function(aMessages) {
> >      let messageList = document.getElementById("message_list");
> >  
> >      // Remove all ignored messages from summarization.
> > +    let summarizedMessages = Array.from(aMessages).filter(msg => !msg.isKilled);
> 
> This seems to me this could be slower/more memory hungry than the original.
> You first push all items into new array and then filter them. The original
> only pushed the matching items.
Switched to |for ... of| with |if (...)| check and push.

> ::: mail/components/devtools/modules/XULRootActor.js
> @@ +171,5 @@
> >  
> >      this._mustNotify = true;
> >      this._checkListening();
> >  
> > +    return Promise.resolve(this._actorByBrowser.map(([_, actor]) => actor));
> 
> Does this work? It seems to me this._actorByBrowser is a Map (which does not
> have .map()), not an array.
Good catch, replaced |.map| with |.forEach|

> ::: mail/components/test/unit/test_about_support.js
> @@ +152,5 @@
> >  function test_get_account_details() {
> >    let accountDetails = AboutSupport.getAccountDetails();
> >    let accountDetailsText = uneval(accountDetails);
> >    // The list of accounts we are looking for
> > +  let accountsToSee = Object.keys(gAccountMap);
> 
> Looks like gAccountMap should be a Map ?
Plot twist: It's an object, let's keep the focus here on fixing, there are many more parts in the code base which uses objects to store key-value pairs from pre-Map times.

chat/im: https://hg.mozilla.org/comm-central/rev/17e3470d8d8d
mail: https://hg.mozilla.org/comm-central/rev/30d7839cccbe
mailnews: https://hg.mozilla.org/comm-central/rev/1a7fafc84e67
I'm not sure why the calendar part was not landed, so there are still many failures on trunk.
But here is a try run with the calendar part, updated per comment 25:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f5230baf3bfa
Comment on attachment 8706339 [details] [diff] [review]
suite, v1

> +  let notifications = [];
> +  for (let i in ar) {
> +    if (ar.slice(0, i).indexOf(ar[i]) == -1) {
> +      notifications(ar[i]);
Shouldn't this be notifications.push(ar[i]);

> -              recipient = [headerParser.makeMimeAddress(fullValue.name, fullValue.email) for (fullValue of headerParser.makeFromDisplayAddress(fieldValue, {}))].join(", ");
> +              recipient = headerParser.makeFromDisplayAddress(fieldValue, {}).map(fullValue => headerParser.makeMimeAddress(fullValue.name, fullValue.email)).join(", ");
I prefer the wrapping in Thunderbird

              recipient =
                headerParser.makeFromDisplayAddress(fieldValue, {}).map(fullValue =>
                  headerParser.makeMimeAddress(fullValue.name, fullValue.email)
              ).join(", ");
Comment on attachment 8706339 [details] [diff] [review]
suite, v1

> -    toremove = [this._byname[coll].title for each (coll in toremove)];
> +    toremove = [];
> +    for each (let coll in toremove) {
> +      toremove.push(this._byname[coll].title);
> +    }
I'm pretty sure this is wrong. Could use Array.map() instead:

toremove = toremove.map(coll => this._byname[coll].title);

> -    return [coll.name for each (coll in this._collections) if (!coll.enabled)];
> +    let names = [];
> +    for each (let coll in this._collections) {
> +      if (!coll.enabled) {
> +        names.push(coll.name);
> +      }
> +    }
> +    return names;

Umm?? Try this instead:

return this._collections.filter(coll => !coll.enabled).map(coll => coll.name);
Attachment #8706339 - Flags: review?(philip.chee) → review-
Comment on attachment 8706339 [details] [diff] [review]
suite, v1

>    let ar = EXPECTED_NOTIFICATIONS.concat(UNEXPECTED_NOTIFICATIONS);
> -  return [ar[i] for (i in ar) if (ar.slice(0, i).indexOf(ar[i]) == -1)];
> +  let notifications = [];
> +  for (let i in ar) {
I don't understand. Shouldn't this shouldn't this be a numeric loop?
  for (let i = 0; i < ar.length; i++)

> +    if (ar.slice(0, i).indexOf(ar[i]) == -1) {
> +      notifications(ar[i]);
> +    }
> +  }
> +  return notifications;

Punting to :arai
Attachment #8706339 - Flags: review?(arai.unmht)
Comment on attachment 8706339 [details] [diff] [review]
suite, v1

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

::: suite/common/places/tests/unit/test_clearHistory_shutdown.js
@@ +133,5 @@
> +    if (ar.slice(0, i).indexOf(ar[i]) == -1) {
> +      notifications(ar[i]);
> +    }
> +  }
> +  return notifications;

so, it does "uniq" operation.
it could be replaced by following:

  return [...new Set(ar)];

or

  return Array.from(new Set(ar));

if Array.from is preferred than spread.
Attachment #8706339 - Flags: review?(arai.unmht)
(In reply to Philip Chee from comment #35)
> > -    return [coll.name for each (coll in this._collections) if (!coll.enabled)];
> > +    let names = [];
> > +    for each (let coll in this._collections) {
> > +      if (!coll.enabled) {
> > +        names.push(coll.name);
> > +      }
> > +    }
> > +    return names;
> 
> Umm?? Try this instead:
> 
> return this._collections.filter(coll => !coll.enabled).map(coll =>
> coll.name);

Note this will run through the array twice, first creating a copy with elements filtered out, second a copy to map to the name. I think the original approach is fine, maybe using for..of instead but that is just nit picking. If you really want to go functional, you could use Array reduce.
(In reply to Philipp Kewisch [:Fallen] from comment #38)
> Note this will run through the array twice, first creating a copy with
> elements filtered out, second a copy to map to the name. I think the
> original approach is fine, maybe using for..of instead but that is just nit
> picking. If you really want to go functional, you could use Array reduce.
The examples I found on MDN and via Google all use a combination of Array.map and Array.filter to replace array comprehensions. I didn't go beyond the first results page on Google though.
Duplicate of this bug: 1239624
Attached patch suite, v2 (obsolete) β€” β€” Splinter Review
Attachment #8706339 - Attachment is obsolete: true
Attachment #8707984 - Flags: review?(philip.chee)
(In reply to Philip Chee from comment #39)
> (In reply to Philipp Kewisch [:Fallen] from comment #38)
> > Note this will run through the array twice, first creating a copy with
> > elements filtered out, second a copy to map to the name. I think the
> > original approach is fine, maybe using for..of instead but that is just nit
> > picking. If you really want to go functional, you could use Array reduce.
> The examples I found on MDN and via Google all use a combination of
> Array.map and Array.filter to replace array comprehensions. I didn't go
> beyond the first results page on Google though.

You can see I had the same issue as Philipp about double iterating an array in my review :)
But that was when iterating messages, where we expect there can be even 1 million of them so we should avoid building large collections of them even temporarily (you can see bugs I filed about how it bogs down TB).

I don't know in your case how many "engines" (whatever it is) there can potentially be in this._collections so whether it is useful to optimize for speed (if a lot) or for simplicity of code (if a few).
Duplicate of this bug: 1238013
Comment on attachment 8707984 [details] [diff] [review]
suite, v2

>  function getDistinctNotifications() {
>    let ar = EXPECTED_NOTIFICATIONS.concat(UNEXPECTED_NOTIFICATIONS);
> -  return [ar[i] for (i in ar) if (ar.slice(0, i).indexOf(ar[i]) == -1)];
> +  let notifications = [];
> +  for (let i = 0; i < ar.length; i++) {
> +    if (!ar.slice(0, i).includes(ar[i])) {
> +      notifications.push(ar[i]);
> +    }
> +  }
> +  return notifications;

As arai says use:
  return [...new Set(ar)];
or
  return Array.from(new Set(ar));

If you want to use Array.filter() StackOverflow has this:
  return ar.filter((value, index, self) => self.indexOf(value) === index;);

(In reply to :aceman from comment #42)
> But that was when iterating messages, where we expect there can be even 1
> million of them so we should avoid building large collections of them even
> temporarily (you can see bugs I filed about how it bogs down TB).
> 
> I don't know in your case how many "engines" (whatever it is) there can
> potentially be in this._collections so whether it is useful to optimize for
> speed (if a lot) or for simplicity of code (if a few).
1. It is highly unlikely that there will be more than 10.
2. This code is going to be refactored anyway.
Attachment #8707984 - Flags: review?(philip.chee) → feedback+
Attached patch suite, v3 β€” β€” Splinter Review
Attachment #8707984 - Attachment is obsolete: true
Attachment #8708034 - Flags: review?(philip.chee)
Comment on attachment 8708034 [details] [diff] [review]
suite, v3

Great! r=me
Attachment #8708034 - Flags: review?(philip.chee) → review+
suite: https://hg.mozilla.org/comm-central/rev/3d833534b10d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Alas! Trunk isn't building (latest builds were on 9-Jan for L32 & L64, 6-Jan for Mac, and 11-Nov 2.42a1 for W32). Let's hope new builds (with the fix) happen soon: in the meantime no Bookmarks Manager (though Ctrl+D and Bookmarks Menu are OK) and no History window.
Confirmed fix OK, on Mac OSX.

Daily 0116 rendering correctly; c.f. dupe bug 1238071.

thanks much.
(In reply to Geoff Lankow from comment #5)
> > [for (foo in bar) foo]
> 
> is a syntax error also, and I found 41 of those in c-c, so that's still
> broken.

But [for (foo of bar) foo] is fine, right? Any reason why [foo for (foo of bar)] wasn't changed to [for (foo of bar) foo]?
(In reply to neil@parkwaycc.co.uk from comment #51)
> But [for (foo of bar) foo] is fine, right? Any reason why [foo for (foo of
> bar)] wasn't changed to [for (foo of bar) foo]?

see the link in comment 6
Depends on: 1240378
Duplicate of this bug: 1240332
Would someone please compile the Win x64 installer, so I can test it on that platform to ensure it works there?
(In reply to Helge Skjeveland from comment #54)
> Would someone please compile the Win x64 installer, so I can test it on that
> platform to ensure it works there?

Adrian, AFAIK the only W64 builds of SeaMonkey 2.43a1 published so far were yours (2015-12-20 https://l10n.mozilla-community.org/~akalla/unofficial/seamonkey/nightly/latest-comm-central-windows64/ ) so could you please check if nothing else prevents building? Also L64 for me if the official Mozilla builders remain stuck?

This might however be impossible as there is IIUC a blocking langpack bug turning the trunk to red.
Flags: needinfo?(akalla)
Confirmed that running the 2015-10-19 TB 46.a01.en-US.win64.installer.exe results in a functional state.
Attached patch Followup to fix iteration over an object (obsolete) β€” β€” Splinter Review
Attachment #8710360 - Flags: review?(florian)
Assignee: aryx.bugmail → aleth
Attachment #8710361 - Flags: review?(florian)
Attachment #8710360 - Attachment is obsolete: true
Attachment #8710360 - Flags: review?(florian)
Attachment #8710361 - Flags: review?(florian) → review+
https://hg.mozilla.org/comm-central/rev/1c297d79a0f1872733b36ec3b88da859655e77a3
Bug 1237602 - Followup to fix iteration over an object. r=florian
Depends on: 1242092
Target Milestone: --- → Thunderbird 46.0
Duplicate of this bug: 1239260
All menus drop down fine on my 2016-1-23 upgraded TB running on Win 7x64.
Assignee: aleth → aryx.bugmail
Duplicate of this bug: 1240485
(In reply to Tony Mechelynck [:tonymec] from comment #55)
> (In reply to Helge Skjeveland from comment #54)
> > Would someone please compile the Win x64 installer, so I can test it on that
> > platform to ensure it works there?
> 
> Adrian, AFAIK the only W64 builds of SeaMonkey 2.43a1 published so far were
> yours (2015-12-20
> https://l10n.mozilla-community.org/~akalla/unofficial/seamonkey/nightly/
> latest-comm-central-windows64/ ) so could you please check if nothing else
> prevents building? Also L64 for me if the official Mozilla builders remain
> stuck?
> 
> This might however be impossible as there is IIUC a blocking langpack bug
> turning the trunk to red.

FYI: I did a new build today from c-c and the installer from there (see link above) seems to work.
Flags: needinfo?(akalla)
So far, as of the 2015-02-05 47.0a1 x64 TB c-c trunk, everything seems to work fine.
(In reply to Adrian Kalla [:adriank] from comment #63)
> FYI: I did a new build today from c-c and the installer from there (see link
> above) seems to work.

Ah, great! DziΔ™kujΔ™ bardzo! This removes the big "Your version is more than four weeks old!" warning which had suddenly appeared yesterday on some SeaMonkey pages:

UA:"Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0 SeaMonkey/2.44a1"
ID:20160208023510 en-US
c-c:4a60c0ffe9af0b68a82a6601c121e251863235e5
m-c:1cfe34ea394c66d7fa2c6dc366b05ab00e919113
(In reply to Tony Mechelynck [:tonymec] from comment #65)
> (In reply to Adrian Kalla [:adriank] from comment #63)
> > FYI: I did a new build today from c-c and the installer from there (see link
> > above) seems to work.
> 
> Ah, great! DziΔ™kujΔ™ bardzo! This removes the big "Your version is more than
> four weeks old!" warning which had suddenly appeared yesterday on some
> SeaMonkey pages:
> 
> UA:"Mozilla/5.0 (X11; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
> SeaMonkey/2.44a1"
> ID:20160208023510 en-US
> c-c:4a60c0ffe9af0b68a82a6601c121e251863235e5
> m-c:1cfe34ea394c66d7fa2c6dc366b05ab00e919113

In the above-mentioned build, duplicate bug 1238400, which is a SeaMonkey bug, is not present anymore: bookmarks window, bookmarks sidebar, and history window are all "populated" again; but since this bug is billed as a Thunderbird bug, I'll leave it to the Thunderbird QA team to complete its VERIFIcation.
Flags: needinfo?(vseerror)
Keywords: verifyme
Depends on: 1247653
No menu problems with recent nightly.
But i also didn't test in the broken period.
Removing verify based on Helge's comment
Flags: needinfo?(vseerror)
Keywords: verifyme
https://hg.mozilla.org/comm-central/rev/5023a71ab95e7c6f792ee48def965d9750116168
Bug 1237602 - Another followup to fix iteration over an object. r=florian
You need to log in before you can comment on or make changes to this bug.