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•10 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•10 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
•