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

RESOLVED FIXED in Thunderbird 46.0

Status

--
blocker
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: aleth, Assigned: aryx)

Tracking

Trunk
Thunderbird 46.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 7 obsolete attachments)

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
(Reporter)

Description

3 years ago
Used in various places (it is of course breaking tests)
(Reporter)

Updated

3 years ago
Blocks: 1220564
Severity: normal → blocker
It seems today Daily is completely broken by bug 1220564. Built with m-c changeset b384917df4e9 all is working.
Posted 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.
(Reporter)

Comment 6

3 years ago
Given bug 1220564 comment 2, it would be better to get rid of comprehensions altogether, especially in dubious cases.
Posted patch 1237602-1.1.diff (obsolete) β€” β€” Splinter Review
Fixes some mistakes in the first patch.
Attachment #8705356 - Attachment is obsolete: true
Posted 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

Comment 10

3 years ago
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
(Reporter)

Comment 11

3 years ago
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

Updated

3 years ago
Duplicate of this bug: 1238400
Duplicate of this bug: 1238071
Posted 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)
Posted patch mail, v1 β€” β€” Splinter Review
Attachment #8706336 - Flags: review?(acelists)
Posted patch mailnews, v1 β€” β€” Splinter Review
Attachment #8706337 - Flags: review?(mkmelin+mozilla)
Posted patch suite, v1 (obsolete) β€” β€” Splinter Review
Attachment #8706339 - Flags: review?(philip.chee)
(Reporter)

Comment 22

3 years ago
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);
(Reporter)

Comment 26

3 years ago
(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 28

3 years ago
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+
Posted 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)
(Reporter)

Comment 30

3 years ago
Comment on attachment 8706632 [details] [diff] [review]
chatim, v2

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

Thanks!
Attachment #8706632 - Flags: review?(aleth) → review+
(Reporter)

Comment 31

3 years ago
(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

Comment 33

3 years ago
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 34

3 years ago
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 35

3 years ago
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 36

3 years ago
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.

Comment 39

3 years ago
(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.

Updated

3 years ago
Duplicate of this bug: 1239624
Posted patch suite, v2 (obsolete) β€” β€” Splinter Review
Attachment #8706339 - Attachment is obsolete: true
Attachment #8707984 - Flags: review?(philip.chee)

Comment 42

3 years ago
(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).

Updated

3 years ago
Duplicate of this bug: 1238013

Comment 44

3 years ago
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+
Posted patch suite, v3 β€” β€” Splinter Review
Attachment #8707984 - Attachment is obsolete: true
Attachment #8708034 - Flags: review?(philip.chee)

Comment 46

3 years ago
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
Last Resolved: 3 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.

Comment 50

3 years ago
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]?
(Reporter)

Comment 52

3 years ago
(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

Updated

3 years ago
Depends on: 1240378
Duplicate of this bug: 1240332

Comment 54

3 years ago
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)

Comment 56

3 years ago
Confirmed that running the 2015-10-19 TB 46.a01.en-US.win64.installer.exe results in a functional state.
(Reporter)

Comment 57

3 years ago
Attachment #8710360 - Flags: review?(florian)
(Reporter)

Updated

3 years ago
Assignee: aryx.bugmail → aleth
(Reporter)

Comment 58

3 years ago
Attachment #8710361 - Flags: review?(florian)
(Reporter)

Updated

3 years ago
Attachment #8710360 - Attachment is obsolete: true
Attachment #8710360 - Flags: review?(florian)
Attachment #8710361 - Flags: review?(florian) → review+
(Reporter)

Comment 59

3 years ago
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

Updated

3 years ago
Duplicate of this bug: 1239260

Comment 61

3 years ago
All menus drop down fine on my 2016-1-23 upgraded TB running on Win 7x64.
(Reporter)

Updated

3 years ago
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)

Comment 64

3 years ago
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
(Reporter)

Updated

3 years ago
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
(Reporter)

Comment 68

3 years ago
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.