Closed
Bug 1237602
Opened 10 years ago
Closed 10 years ago
Port Bug 1220564 - Remove legacy array/generator comprehension for comm-central
Categories
(Thunderbird :: General, defect)
Thunderbird
General
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)
Comment 1•10 years ago
|
||
It seems today Daily is completely broken by bug 1220564. Built with m-c changeset b384917df4e9 all is working.
Comment 2•10 years ago
|
||
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.
Comment 3•10 years ago
|
||
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 4•10 years ago
|
||
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)
Comment 5•10 years ago
|
||
> [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•10 years ago
|
||
Given bug 1220564 comment 2, it would be better to get rid of comprehensions altogether, especially in dubious cases.
Comment 7•10 years ago
|
||
Fixes some mistakes in the first patch.
Attachment #8705356 -
Attachment is obsolete: true
Comment 8•10 years ago
|
||
More stuff I've found since the first patch.
Comment 9•10 years ago
|
||
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.
Updated•10 years ago
|
Assignee: geoff → nobody
Status: ASSIGNED → NEW
![]() |
||
Comment 10•10 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•10 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"
![]() |
Assignee | |
Comment 12•10 years ago
|
||
> I can look at this today. But if anybody is interested too, we can split the work.
Generating the file lists at the moment.
![]() |
Assignee | |
Comment 17•10 years ago
|
||
Assignee: nobody → aryx.bugmail
Attachment #8705504 -
Attachment is obsolete: true
Attachment #8705505 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8706335 -
Flags: review?(aleth)
![]() |
Assignee | |
Comment 18•10 years ago
|
||
Attachment #8706336 -
Flags: review?(acelists)
![]() |
Assignee | |
Comment 19•10 years ago
|
||
Attachment #8706337 -
Flags: review?(mkmelin+mozilla)
![]() |
Assignee | |
Comment 20•10 years ago
|
||
Attachment #8706339 -
Flags: review?(philip.chee)
![]() |
Assignee | |
Comment 21•10 years ago
|
||
Attachment #8706343 -
Flags: review?(philipp)
Reporter | ||
Comment 22•10 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 23•10 years ago
|
||
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 24•10 years ago
|
||
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);
Comment 25•10 years ago
|
||
or even let attendeeData = aItem.getAttendees({}).map(createAttendee);
Reporter | ||
Comment 26•10 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 27•10 years ago
|
||
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•10 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+
![]() |
Assignee | |
Comment 29•10 years ago
|
||
(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•10 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•10 years ago
|
||
(In reply to Sebastian H. [:aryx][:archaeopteryx] from comment #29)
> Unfortunately, Object.values is only enabled on Nightly.
oh, good catch!
![]() |
Assignee | |
Comment 32•10 years ago
|
||
(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•10 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•10 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•10 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•10 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 37•10 years ago
|
||
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)
Comment 38•10 years ago
|
||
(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•10 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.
![]() |
Assignee | |
Comment 41•10 years ago
|
||
Attachment #8706339 -
Attachment is obsolete: true
Attachment #8707984 -
Flags: review?(philip.chee)
![]() |
||
Comment 42•10 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).
![]() |
||
Comment 44•10 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+
![]() |
Assignee | |
Comment 45•10 years ago
|
||
Attachment #8707984 -
Attachment is obsolete: true
Attachment #8708034 -
Flags: review?(philip.chee)
![]() |
||
Comment 46•10 years ago
|
||
Comment on attachment 8708034 [details] [diff] [review]
suite, v3
Great! r=me
Attachment #8708034 -
Flags: review?(philip.chee) → review+
![]() |
Assignee | |
Comment 47•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 48•10 years ago
|
||
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 49•10 years ago
|
||
P.S. Trunk SeaMonkey I mean.
Comment 50•10 years ago
|
||
Confirmed fix OK, on Mac OSX.
Daily 0116 rendering correctly; c.f. dupe bug 1238071.
thanks much.
Comment 51•10 years ago
|
||
(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•10 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
Comment 54•10 years ago
|
||
Would someone please compile the Win x64 installer, so I can test it on that platform to ensure it works there?
Comment 55•10 years ago
|
||
(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•10 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•10 years ago
|
||
Attachment #8710360 -
Flags: review?(florian)
Reporter | ||
Updated•10 years ago
|
Assignee: aryx.bugmail → aleth
Reporter | ||
Comment 58•10 years ago
|
||
Attachment #8710361 -
Flags: review?(florian)
Reporter | ||
Updated•10 years ago
|
Attachment #8710360 -
Attachment is obsolete: true
Attachment #8710360 -
Flags: review?(florian)
Updated•10 years ago
|
Attachment #8710361 -
Flags: review?(florian) → review+
Reporter | ||
Comment 59•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/1c297d79a0f1872733b36ec3b88da859655e77a3
Bug 1237602 - Followup to fix iteration over an object. r=florian
Updated•10 years ago
|
Target Milestone: --- → Thunderbird 46.0
Comment 61•10 years ago
|
||
All menus drop down fine on my 2016-1-23 upgraded TB running on Win 7x64.
Reporter | ||
Updated•10 years ago
|
Assignee: aleth → aryx.bugmail
Comment 63•10 years ago
|
||
(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•10 years ago
|
||
So far, as of the 2015-02-05 47.0a1 x64 TB c-c trunk, everything seems to work fine.
Comment 65•10 years ago
|
||
(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
Comment 66•10 years ago
|
||
(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
Comment 67•9 years ago
|
||
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•9 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.
Description
•