1401528 - Massive Xpcshell and Mozmill test failure 2017-09-20: 55 failing Xpcshell tests, 88 failing Mozmill tests

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_emptyTrash_dbViewWrapper.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_imapFolder.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_virtualFolderCustomTerm.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TEST-UNEXPECTED-FAIL | mailnews/imap/test/unit/test_imapFilterActions.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TypeError: Ci is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_imapFilterActions.js:15 [log…]
TypeError: gTestArray is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_imapFilterActions.js:333 [log…]
TEST-UNEXPECTED-FAIL | mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TypeError: Ci is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js:13 [log…]
TypeError: gTestArray is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js:182 [log…]
TEST-UNEXPECTED-FAIL | mailnews/imap/test/unit/test_offlineDraftDataloss.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TypeError: Ci is undefined at ../../../resources/logHelper.js:136 [log…]
ReferenceError: IMAPPump is not defined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_offlineDraftDataloss.js:122 [log…]
TEST-UNEXPECTED-FAIL | mailnews/imap/test/unit/test_offlinePlayback.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
ReferenceError: setupIMAPPump is not defined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_offlinePlayback.js:19 [log…]
TypeError: tests is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_offlinePlayback.js:139 [log…]
TEST-UNEXPECTED-FAIL | mailnews/imap/test/unit/test_imapFilterActions.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TypeError: Ci is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_imapFilterActions.js:15 [log…]
TypeError: gTestArray is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_imapFilterActions.js:333 [log…]
TEST-UNEXPECTED-FAIL | mailnews/imap/test/unit/test_offlineDraftDataloss.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
TypeError: Ci is undefined at ../../../resources/logHelper.js:136 [log…]
ReferenceError: IMAPPump is not defined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_offlineDraftDataloss.js:122 [log…]
TEST-UNEXPECTED-FAIL | mailnews/imap/test/unit/test_offlinePlayback.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
ReferenceError: setupIMAPPump is not defined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_offlinePlayback.js:19 [log…]
TypeError: tests is undefined at C:/slave/test/build/tests/xpcshell/tests/mailnews/imap/test/unit/test_offlinePlayback.js:139 [log…]
TEST-UNEXPECTED-FAIL | mailnews/news/test/unit/test_bug695309.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
ReferenceError: nntpDaemon is not defined at C:/slave/test/build/tests/xpcshell/tests/mailnews/news/test/unit/head_server_setup.js:45 [log…]
TEST-UNEXPECTED-FAIL | mailnews/news/test/unit/test_server.js | xpcshell return code: 0 [log…]
ReferenceError: assignment to undeclared variable TextDecoder at resource:///modules/jsmime.jsm:66 [log…]
ReferenceError: nntpDaemon is not defined at C:/slave/test/build/tests/xpcshell/tests/mailnews/news/test/unit/head_server_setup.js:45 [log…]
ReferenceError: NNTP_RFC977_handler is not defined at C:/slave/test/build/tests/xpcshell/tests/mailnews/news/test/unit/test_server.js:30 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_alarm.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_attendee.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_attachment.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug350845.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug272411.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_ics.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug1209399.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug1204255.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug1199942.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug486186.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug485571.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_providers.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_gdata_provider.js | xpcshell return code: 0 [log…]
TypeError: this.Array is undefined at resource://gdata-provider/modules/gdataSession.jsm:517 [log…]
ReferenceError: MockRegistrar is not defined at C:/slave/test/build/tests/xpcshell/tests/calendar/test/unit/test_gdata_provider.js:66 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_imip.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_relation.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug668222.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug494140.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug759324.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_view_utils.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug523860.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug653924.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_hashedarray.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_datetime_before_1970.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_duration.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_calutils.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | xpcshell-icaljs.ini:calendar/test/unit/test_calutils.js | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_calitiputils.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | xpcshell-icaljs.ini:calendar/test/unit/test_calitiputils.js | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:calendar/test/unit/test_gdata_provider.js | xpcshell return code: 0 [log…]
TypeError: this.Array is undefined at resource://gdata-provider/modules/gdataSession.jsm:517 [log…]
ReferenceError: MockRegistrar is not defined at C:/slave/test/build/tests/xpcshell/tests/calendar/test/unit/test_gdata_provider.js:66 [log…]
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:calendar/test/unit/test_calutils.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | xpcshell-libical.ini:calendar/test/unit/test_calutils.js | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | xpcshell-libical.ini:calendar/test/unit/test_calitiputils.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | xpcshell-libical.ini:calendar/test/unit/test_calitiputils.js | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | chat/components/src/test/test_accounts.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | chat/protocols/xmpp/test/test_parseJidAndNormalization.js | xpcshell return code: 0 [log…]
TypeError: this.__defineGetter__ is not a function at resource:///modules/xmpp-session.jsm:24 [log…]
TEST-UNEXPECTED-FAIL | chat/protocols/xmpp/test/test_dnsSrv.js | xpcshell return code: 0 [log…]
TypeError: this.__defineGetter__ is not a function at resource:///modules/xmpp-session.jsm:24 [log…]
TypeError: TEST_DATA is undefined at C:/slave/test/build/tests/xpcshell/tests/chat/protocols/xmpp/test/test_dnsSrv.js:106 [log…]
TEST-UNEXPECTED-TIMEOUT | xpcshell-icaljs.ini:calendar/test/unit/test_timezone.js | Test timed out [log…]
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_weekinfo_service.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_alertHook.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_virtualFolder.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | mail/base/test/unit/test_viewWrapper_virtualFolder.js | application crashed [@ nsMsgSearchTerm::MatchRfc2047String(nsTSubstring<char> const &,char const *,bool,bool *)] [log…]
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_logic.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | mail/base/test/unit/test_viewWrapper_logic.js | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | mailnews/addrbook/test/unit/test_nsAbAutoCompleteSearch1.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | mailnews/addrbook/test/unit/test_nsAbAutoCompleteSearch2.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_realFolder.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | mailnews/addrbook/test/unit/test_collection_2.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | mailnews/addrbook/test/unit/test_collection_2.js | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | mailnews/addrbook/test/unit/test_nsAbAutoCompleteSearch3.js | xpcshell return code: 0 [log…]
TEST-UNEXPECTED-FAIL | mailnews/addrbook/test/unit/test_collection.js | xpcshell return code: 1 [log…]
PROCESS-CRASH | mailnews/addrbook/test/unit/test_collection.js | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | mail/base/test/unit/test_viewWrapper_virtualFolder.js | xpcshell return code: 1 [log…] 

TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-ab-whitelist.js | test-ab-whitelist.js::test_address_book_whitelist [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-deletion.js | test-account-deletion.js::test_account_data_deletion [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-deletion.js | test-account-deletion.js::teardownModule [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-settings-infrastructure.js | test-account-settings-infrastructure.js::test_account_dot_IDs [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-settings-infrastructure.js | test-account-settings-infrastructure.js::test_account_locked_prefs [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-settings-infrastructure.js | test-account-settings-infrastructure.js::test_account_onchange_handler [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-values.js | test-account-values.js::test_default_CC_addressJavaScript error: resource:///modules/jsmime.jsm, line 66: ReferenceError: assignment to undeclared variable TextDecoder [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-values.js | test-account-values.js::test_invalid_junk_target [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-account-values.js | test-account-values.js::test_invalid_hostname [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-archive-options.js | test-archive-options.js::test_archive_options_enabled [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-archive-options.js | test-archive-options.js::test_archive_enabled [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\account\test-archive-options.js | test-archive-options.js::test_disable_archive [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\addrbook\test-address-book.js | test-address-book.js::test_writing_to_mailing_list [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-events.js | test-attachment-events.js::test_attachments_added_on_single [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-events.js | test-attachment-events.js::test_attachments_added_on_multiple [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-events.js | test-attachment-events.js::test_attachments_removed_on_single [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-events.js | test-attachment-events.js::test_attachments_removed_on_multiple [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-events.js | test-attachment-events.js::test_no_attachments_removed_on_none [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-events.js | test-attachment-events.js::test_attachment_renamed [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\attachment\test-attachment-events.js | test-attachment-events.js::test_no_attachment_renamed_on_blank [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | attachment | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-item.js | test-cloudfile-attachment-item.js::test_upload_cancel_repeat [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\cloudfile\test-cloudfile-attachment-item.js | test-cloudfile-attachment-item.js::test_upload_multiple_and_cancel [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | cloudfile | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-charset-edit.js | test-charset-edit.js::<TOP_LEVEL> [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-multipart-related.js | test-multipart-related.js::<TOP_LEVEL> [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-address-widgets.js | test-address-widgets.js::test_address_types [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment-reminder.js | test-attachment-reminder.js::setupModule [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_file_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_webpage_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_multiple_attachments [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_delete_attachments [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_rename_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_open_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_forward_raw_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_forward_b64_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_forward_message_as_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-attachment.js | test-attachment.js::test_forward_message_with_attachments_as_attachment [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-blocked-content.js | test-blocked-content.js::setupModule [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-charset-edit.js | test-charset-edit.js::setupModule [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\composition\test-charset-upgrade.js | test-charset-upgrade.js::setupModule [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | composition | application crashed [@ mozilla::mailnews::MakeMimeAddress(nsTSubstring<char16_t> const &,nsTSubstring<char16_t> const &,nsTSubstring<char16_t> &)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | content-policy | application crashed [@ nsMsgCompFields::SetAsciiHeader(nsMsgCompFields::MsgHeaderID,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | content-tabs | application crashed [@ mozilla::mailnews::MakeMimeAddress(nsTSubstring<char16_t> const &,nsTSubstring<char16_t> const &,nsTSubstring<char16_t> &)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | downloads | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
[INCOMPLETE] 1368783 - Intermittent toolkit/components/extensions/test/xpcshell/test_ext_downloads_misc.js | application crashed [@ mozalloc_abort(char const * const)] [ 0.34 ]
[FIXED] 1376668 - Intermittent toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_open.html | application crashed [@ EnqueueTask] [ 0.30 ]
1379484 - Intermittent browser/components/extensions/test/xpcshell/test_ext_browsingData_downloads.js | application crashed [@ mozilla::gfx::DeviceManagerDx::CreateMLGDevice()] after Assertion failure: layers::CompositorThreadHolder::IsInCompositorThread() [ 0.30 ]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | folder-display | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | folder-pane | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-tree-modes\test-custom-smart-folder.js | test-custom-smart-folder.js::test_custom_folder_exists [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-tree-modes\test-smart-folders.js | test-smart-folders.js::test_get_folder_for_msg_hdr [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-tree-modes\test-smart-folders.js | test-smart-folders.js::test_select_folder_expands_collapsed_smart_inbox [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-tree-modes\test-smart-folders.js | test-smart-folders.js::test_select_folder_expands_collapsed_account_root [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-tree-modes\test-smart-folders.js | test-smart-folders.js::test_folder_flag_changes [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-tree-modes\test-unread-folders.js | test-unread-folders.js::test_switch_to_all_folders [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\folder-widget\test-message-filters.js | test-message-filters.js::setupModule [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\im\test-toolbar-buttons.js | test-toolbar-buttons.js::test_toolbar_and_placeholder [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\invitations\test-imip-bar-eml.js | test-imip-bar-eml.js::test_event_from_eml [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | junk-commands | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | keyboard | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | message-header | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed [log…]
PROCESS-CRASH | message-window | application crashed [@ mozilla::mailnews::EncodedHeader(nsTSubstring<char> const &,char const *)] [log…]
TEST-UNEXPECTED-FAIL | (runtestlist.py) | Exited with code 1 during directory run [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_single_identity [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_single_identity_in_abook [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_single_identity_in_abook_no_pdn [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_multiple_identities [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_multiple_identities_in_abook [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_multiple_identities_in_abook_no_pdn [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_no_header_name [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_no_header_name_in_abook [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_no_header_name_in_abook_no_pdn [log…]
TEST-UNEXPECTED-FAIL | C:\slave\test\build\tests\mozmill\multiple-identities\test-display-names.js | test-display-names.js::test_header_name [log…] 

M-C last good: a0eb21bf55e1c1ae0ba311e6f2273da05c
M-C first bad: a20de99fa3c1ba6287fe47d493a859a4e9
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a0eb21bf55e1c1ae0ba311e6f2273da05c&tochange=a20de99fa3c1ba6287fe47d493a859a4e9

I can't see it in a hurry, Arai, can you?

Looks like it's raising many JS errors all of a sudden.

Comment 1

[Tooru Fujisawa [:arai]]

maybe this?
https://hg.mozilla.org/mozilla-central/rev/cf450ba78f02

flipping jsloader.shareGlobal to false may help
(of course it would be a temporary workaround)

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-jsloader.patch [landed: comment #3]

This fixes a test I tried, so let's hope the other ones will pass as well.

Arai: Thank you, thank you, thank you!!!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4de5f6029108
set pref jsloader.shareGlobal to false to avoid test failures. rs=bustage-fix

Comment 4

[Jorg K (CEST = GMT+2)]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> flipping jsloader.shareGlobal to false may help
> (of course it would be a temporary workaround)
Thanks again, that worked. So what would be the proper fix?

Comment 5

[Tooru Fujisawa [:arai]]

changes around bug 1381961 should be the necessary for it.
like bug 1399997. bug 1400566, etc
so, code that depends on global object needs fix, so that it retrieves correct global object.

also, this should affect extension that uses JSM.

Comment 6

[Jorg K (CEST = GMT+2)]

Aceman, this looks like an ample field for activity.

Comment 7

[Jorg K (CEST = GMT+2)]

See bug 1401897 comment #14:
+var TextDecoder = Components.utils.getGlobalForObject(
+  Components.utils.import("resource://gre/modules/Services.jsm", {}));
+
 var RealTextDecoder = TextDecoder;
+
for jsmime.js inspired by
https://hg.mozilla.org/mozilla-central/rev/9109b1f49d23#l3.13

Comment 8

[Jorg K (CEST = GMT+2)]

Maybe that's not right, we don't want to declare another TextDecoder but use the one from the module.

Perhaps more like this:
https://hg.mozilla.org/mozilla-central/rev/9109b1f49d23#l5.13
or this:
https://hg.mozilla.org/mozilla-central/rev/9109b1f49d23#l6.11

Comment 9

[Jorg K (CEST = GMT+2)]

const {TextDecoder} = Components.utils.getGlobalForObject(
  Components.utils.import("resource://gre/modules/Services.jsm", {}));
or without the const?

Comment 10

[Ian Neal]

As mentioned in another bug.

so does https://dxr.mozilla.org/comm-central/source/mailnews/mime/src/jsmime.jsm#6 need changing to be:
const jsmScope = Components.utils.import("resource://gre/modules/Services.jsm", {});

then add lines:

const { Services } = jsmScope;
const { TextDecoder } = Components.utils.getGlobalForObject(jsmScope);

Note this is in jsmime.jsm not jsmime.js

Comment 11

[Frank-Rainer Grahl (:frg)]

Attachment: 1401528-textdecoder.patch

This one seems to work. I left 2 log messages in the patch and RealTextDecoder does not cause an exception in my SeaMonkey 2.55 build. The services call is still a bit voodoo to me because TexDecoder is not in Services.jsm :)

If you think this ok please set review or I upload a new one and remove the log messages.

Comment 12

[Jorg K (CEST = GMT+2)]

I thinks it's OK, but we need to fix all errors from comment #0. We could land this as "part 1" if it helps you.

Comment 13

[Jorg K (CEST = GMT+2)]

I'm giving this a spin with pref("jsloader.shareGlobal", true);
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=86f33c2e7d20b028d14051423db2b70612011fbe

I'd like to know how many test failures go away. I assume we'll still have to fix all the remaining ReferenceErrors and TypeErrors from comment #0. I'll post a new summary when the try run finishes.

Comment 14

[:aceman]

Are any of the patches already working? E.g. the jsloader patch does not apply for me.

Comment 15

[Jorg K (CEST = GMT+2)]

Yes, jsloader landed as fix bustage, now looking into fixing the errors for real. FRG's patch should reduce the errors greatly, see comment #13. You can give me a hand to fix the remaining errors.

Comment 16

[:aceman]

Ok, I looked at the mailnews/imap/test/unit/test_imapFilterActions.js problems. After setting jsloader.shareGlobal to true again, 
I got all the 3 errors in it, as in comment 0 (TextDecoder, Ci, gTestArray). After applying 1401528-textdecoder.patch I get no problems. So that looks positive, some of the successive errors are caused by the TextDecoder failure.

When running all of /imap/ tests, I get
JavaScript strict warning: resource:///modules/jsmime.jsm -> resource:///modules/jsmime/jsmime.js, line 2606: ReferenceError: reference to undefined property "softMargin""
in some tests.

Comment 17

[:aceman]

If options.softMargin is optional HeaderEmitter(), we need to handle it cleanly. In many cases in the code we call HeaderEmitter() with empty 'options' object.

Comment 18

[Jorg K (CEST = GMT+2)]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

Joshua, the action is here now.

Aceman, please look at
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=132956300
Much better with Mozmill almost green.

Comment 19

[:aceman]

Attachment: 1401528-HeaderEmitter.patch

This helps several tests to pass.

Comment 20

[:aceman]

(In reply to Jorg K (GMT+2) from comment #18)
> Aceman, please look at
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&selectedJob=132956300
> Much better with Mozmill almost green.

Yes nice, the TextDecoder helped.

Now I get this in the remaining IMAP xpcshell:
JavaScript Warning: "ReferenceError: reference to undefined property "processType"" {file: "file:///tbird-bin/dist/bin/components/nsLoginManager.js" line: 98}]"
JavaScript Warning: "ReferenceError: reference to undefined property "processType"" {file: "resource://gre/modules/AsyncShutdown.jsm" line: 71}]"
JavaScript Warning: "ReferenceError: reference to undefined property "processType"" {file: "resource://gre/modules/osfile/osfile_async_front.jsm" line: 1421}]"

Comment 21

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

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

::: mailnews/mime/src/jsmime.jsm
@@ +54,5 @@
>    },
>  };
>  
> +var {TextDecoder} = Components.utils.getGlobalForObject(
> +  Components.utils.import("resource://gre/modules/Services.jsm", {}));

As I suggested in the other bug, saying

var RealTextDecoder = this.TextDecoder;

and

this.TextDecoder = FallbackTextDecoder;

should work, as it grabs the TextDecoder rather than getting it from the global of another object.

Comment 22

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8911615 [details] [diff] [review]
1401528-HeaderEmitter.patch

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

It's better to change clamp so that it takes an object and the name instead of doing the check in the caller. i.e., the call would be
clamp(options, 'softMargin', 30, 900, 78)

Comment 23

[Joshua Cranmer [:jcranmer]]

Actually, looking back at the code, it's not clear that the jsmime.jsm code is working correctly. We used to have a test for UTF-7 support in RFC 2047 encoding in test_jsmime_charset.js (whose sole purpose was to make sure non-encoding-spec charsets, i.e., UTF-7, were supported). That was apparently temporarily removed in bug 1363281, but bug 1372899 failed to re-add it. This means that there are no longer any tests in the tree that are actually checking the correctness of the code in question.

r+ for the code in question is therefore predicated on re-adding those tests and ensuring that they pass.

Comment 24

[Jorg K (CEST = GMT+2)]

When you're referring to removing tests, you mean this, right?
https://hg.mozilla.org/comm-central/rev/322986aa2dbabda0ba031d76ba2dc959c7cc9f7a#l3.12
 var tests = [
-  ["=?UTF-7?Q?+AKM-1?=", "\u00A31"],
-  ["=?UTF-7?Q?+AK?= =?UTF-7?Q?M-1?=", "\u00A31"],

I don't quite understand the trickery that JSMime does with the decoders. You set TextDecoder = FallbackTextDecoder, and the FallbackTextDecoder is either the original/real TextDecoder or a FakeTextDecoder which uses nsIScriptableUnicodeConverter.

The latter stopped supporting UTF-7 and the original/real TextDecoder doesn't support it either.

The only (M)UTF-7 support we have in all of M-C and C-C is in nsMsgI18N.cpp added and reviewed you yourself:
https://hg.mozilla.org/comm-central/rev/ac30eed3d7e54d44f6824d9da864d3a89579e76c#l1.32

That's not integrated with any text decoder, so I think UTF-7 support in RFC 2047 encoding was discontinued for good (as much as message body encoding in UTF-7), and I don't understand which tests you want to re-establish.

Comment 25

[Joshua Cranmer [:jcranmer]]

(In reply to Jorg K (GMT+2) from comment #24)
> When you're referring to removing tests, you mean this, right?
> https://hg.mozilla.org/comm-central/rev/
> 322986aa2dbabda0ba031d76ba2dc959c7cc9f7a#l3.12
>  var tests = [
> -  ["=?UTF-7?Q?+AKM-1?=", "\u00A31"],
> -  ["=?UTF-7?Q?+AK?= =?UTF-7?Q?M-1?=", "\u00A31"],

Yes.

> I don't quite understand the trickery that JSMime does with the decoders.
> You set TextDecoder = FallbackTextDecoder, and the FallbackTextDecoder is
> either the original/real TextDecoder or a FakeTextDecoder which uses
> nsIScriptableUnicodeConverter.

The basic trickery is this:

jsmime uses TextDecoder to convert charsets. TextDecoder in the encoding-spec doesn't support UTF-7 (the polyfill for node.js uses iconv, which supports UTF-7). We need to support UTF-7, so we provide our own copy of TextDecoder. This new version of TextDecoder will try to use the original TextDecoder (saved under the name RealTextDecoder). If the charset isn't recognized (the constructor throws an error), we use a different variant of TextDecoder that calls FakeTextDecoder for other charsets (basically UTF-7 at this point).

> The latter stopped supporting UTF-7 and the original/real TextDecoder
> doesn't support it either.

Then that is a regression that needs to be fixed.

> The only (M)UTF-7 support we have in all of M-C and C-C is in nsMsgI18N.cpp
> added and reviewed you yourself:

I said in that bug:
> On the other hand, we are going to need to directly call the UTF-7 decoder from somewhere for jsmime and libmime, and keeping the similarity of code for these pieces may make some sense. I don't exactly see where you're triggering the UTF-7 decoder in such a way that we can decode UTF-7 text/ parts or UTF-7 in RFC 2047 encoded-words, so I can't comment on what work is necessary.

I only reviewed one of the pieces of the multi-part bug, and my understanding was the work would be added for a later piece. Indeed, I even lamented that we didn't have tests for UTF-7 in the body portion of the message. Evidently, some wires were crossed in communication, but it was never intended by me that UTF-7 support here be dropped and I would not have accepted a patch that did that.

Comment 26

[:aceman]

Attachment: 1401528-HeaderEmitter.patch v2 [landed: comment #60]

Comment 27

[Jorg K (CEST = GMT+2)]

(In reply to Joshua Cranmer [:jcranmer] from comment #23)
> r+ for the code in question is therefore predicated on re-adding those tests
> and ensuring that they pass.
I beg to differ. I filed bug 1402813 to re-establish UTF-7 support in JS Mime but we should fix the JS issues encountered here independently as soon as possible. UTF-7 support was removed in TB 56 and no one has complained through the beta cycle. Whilst it would be good to re-establish it, we shouldn't risk severe bustage by not fixing this bug here as soon as possible.

Comment 28

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-textdecoder2.patch - NOT WORKING (see comment 31 and 32)

mailnews/imap/test/unit/test_imapFilterActions.js passes with this patch.

Comment 29

[Jorg K (CEST = GMT+2)]

Aceman, would you be able to take this bug. I've just attached the patch for jsmime.jsm that reflects our discussion.

Comment 30

[Joshua Cranmer [:jcranmer]]

(In reply to Jorg K (GMT+2) from comment #27)
> I beg to differ. I filed bug 1402813 to re-establish UTF-7 support in JS
> Mime but we should fix the JS issues encountered here independently as soon
> as possible. UTF-7 support was removed in TB 56 and no one has complained
> through the beta cycle. Whilst it would be good to re-establish it, we
> shouldn't risk severe bustage by not fixing this bug here as soon as
> possible.

Since the code was unexpectedly broken and this is blocking getting lots of tests passing, I will agree here.

Comment 31

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8911733 [details] [diff] [review]
1401528-textdecoder2.patch - NOT WORKING (see comment 31 and 32)

I tried this patch with my console outputs still in and it seems to always call FakeTextDecoder after RealTextdecoder which indicates that RealTextDecoder throws an exception. Is the headerEmitter patch mandatory too?

Comment 32

[Jorg K (CEST = GMT+2)]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

Hmm, setting jsloader.shareGlobal to true and running with the "this." patch indeed uses the fake decoder.

When using FRG's version of
+var {TextDecoder} = Components.utils.getGlobalForObject(
+  Components.utils.import("resource://gre/modules/Services.jsm", {}));
that doesn't happen.

Arai, can you enlighten us here please.

Comment 33

[Jorg K (CEST = GMT+2)]

Comment on attachment 8911733 [details] [diff] [review]
1401528-textdecoder2.patch - NOT WORKING (see comment 31 and 32)

About this version, Joshua said in bug 1401897 comment #32:
"Replacing TextDecoder with this.TextDecoder in jsmime.jsm should work, I think?"

That doesn't sound 100% confident.

Comment 34

[:aceman]

The last mozmill failure in mozmill\im\test-toolbar-buttons.js may be caused by this:

JavaScript error: file:///tbird-bin/dist/bin/components/imContacts.js, line 99: TypeError: this.__defineGetter__ is not a function
[28921, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, NS_ERROR_XPC_GS_RETURNED_FAILURE) failed with result 0x80040154: file mozilla/js/xpconnect/src/XPCJSID.cpp, line 702
[Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: resource://gre/modules/XPCOMUtils.jsm
 :: XPCU_serviceLambda :: line 266"  data: no] ({})
JavaScript error: file:///tbird-bin/dist/bin/components/imCore.js, line 270: TypeError: Services.contacts is undefined
JavaScript error: resource:///modules/chatHandler.jsm, line 37: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "Services.contacts is undefined" {file: "file:///tbird-bin/dist/bin/components/imCore.js" line:
270}]'[JavaScript Error: "Services.contacts is undefined" {file: "file:///tbird-bin/dist/bin/components/imCore.js" line: 270}]' when calling method: [imICoreService::init]
JavaScript error: file:///tbird-bin/dist/bin/components/imConversations.js, line 494: TypeError: this._prplConversations is undefined
JavaScript error: file:///tbird-bin/dist/bin/components/imCore.js, line 293: NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "this._prplConversations is undefined" {file: "file:///tbird-bin/dist
/bin/components/imConversations.js" line: 494}]'[JavaScript Error: "this._prplConversations is undefined" {file: "file:///tbird-bin/dist/bin/components/imConversations.js" line: 494}]' when calling method: [imIConversations
Service::unInitConversations]

Comment 35

[:aceman]

There is an interesting error in calendar/test/unit/test_gdata_provider.js
JavaScript strict warning: resource://gdata-provider/modules/gdataSession.jsm, line 517: ReferenceError: reference to undefined property "Array"
TypeError: this.Array is undefined at resource://gdata-provider/modules/gdataSession.jsm:517

The code on this line is some obfuscated JS. Maybe I could decode it in some way but there is a big fat warning not to do it.
Fallen can you look at that first?

Comment 36

[:aceman]

Attachment: 1401528-gloda.patch

This fixes the errors found in gloda tests:
INFO -  "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError: reference to undefined property "special"" {file: "resource:///modules/gloda/gloda.js" line: 1512}]"
INFO -  Error console says [stackFrame ReferenceError: reference to undefined property "special"]

However, after this patch I still get stuck at this error:
JavaScript strict warning: resource:///modules/gloda/gloda.js, line 1821: ReferenceError: reference to undefined property Symbol.toPrimitive"

I don't know what that means, but the code is:
this._attrProvidersByNoun[subjectType][aAttrDef.provider].push(aAttrDef);

and this error happens when subjectType is 101, which is the first value when this code is hit. Then with values of 102 and 103 there is no message.
Asuth, do you know why is 101 special?

Comment 37

[:aceman]

Attachment: 1401528-smtpd.patch [landed: comment #60]

This is for the error that kills some compose+smtp tests:
JavaScript strict warning: resource://testing-common/mailnews/smtpd.js, line 79: ReferenceError: reference to undefined property 1

Comment 38

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8911947 [details] [diff] [review]
1401528-gloda.patch

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

Thanks for keeping Thunderbird and gloda working!

(In reply to :aceman from comment #36)
> This fixes the errors found in gloda tests:
> INFO -  "CONSOLE_MESSAGE: (warn) [JavaScript Warning: "ReferenceError:
> reference to undefined property "special"" {file:
> "resource:///modules/gloda/gloda.js" line: 1512}]"
> INFO -  Error console says [stackFrame ReferenceError: reference to
> undefined property "special"]

This sounds like gloda is getting opted into strict mode by the global sharing changes or something similar.  Your changes here work for avoiding that getting mad.
 
> However, after this patch I still get stuck at this error:
> JavaScript strict warning: resource:///modules/gloda/gloda.js, line 1821:
> ReferenceError: reference to undefined property Symbol.toPrimitive"
> 
> I don't know what that means, but the code is:
> this._attrProvidersByNoun[subjectType][aAttrDef.provider].push(aAttrDef);
> 
> and this error happens when subjectType is 101, which is the first value
> when this code is hit. Then with values of 102 and 103 there is no message.
> Asuth, do you know why is 101 special?

Going by https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/toPrimitive I believe what is happening is there's a bug.  [aAttrDef.provider] in that line should be [aAttrDef.provider.providerName], as it should also be two lines up.

What's happening right now is that we're trying to subscript an Object with another Object, which isn't really a reasonable thing to do.  The runtime is trying to use toPrimitive to figure out how to coerce the object to a primitive which is valid.  So it looks for the magic symbol to help it.  And I guess it only does it once.

The reason this doesn't cause any real problems is that this._attrProviderOrderByNoun is the thing we use for iteration.  this._attrProvidersByNoun is apparently extra book-keeping I never got around to using.  It's reported as a warning so it shouldn't actually be causing gloda problems.

But certainly rs=asuth to perform the fix-up I note above.

I've largely filtered Thunderbird bugzilla out, so please be sure to set another review/feedback flag or needinfo if you need more input.

Comment 39

[:aceman]

For the reviewers, this is a try run where the errors can be seen (it is only with textdecoder+headerEmitter patches applied):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=89983c8019e1aad7cce2a3a302464c87f4fbe5e1

New run with all the patches applied:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f31bb39cb9a4e827bce4e64cdcb998e18331cc16

Still remaining are the "processType" errors and some errors with addons.

Comment 40

[:aceman]

Attachment: 1401528-gloda.patch v2 [landed: comment #60]

Thanks asuth, that helped. The tests proceeded and uncovered some more problems, so I added more fixes. Please look at this for the last time.
After this patch about 10 tests pass and 7 still fail on the already known (but unsolved) references to processType, that we probably need to fix for all tests at some central place.

Comment 41

[:aceman]

For
'ReferenceError: MockRegistrar is not defined at C:/slave/test/build/tests/xpcshell/tests/calendar/test/unit/test_gdata_provider.js:66'
I tried to change to
const {MockRegistrar} = Components.utils.import("resource://testing-common/MockRegistrar.jsm", {});
but that didn't help much (error changed to "ReferenceError: can't access lexical declaration `MockRegistrar' before initialization").

Comment 42

[:aceman]

(In reply to :aceman from comment #34)
> 270}]'[JavaScript Error: "Services.contacts is undefined" {file:
> "file:///tbird-bin/dist/bin/components/imCore.js" line: 270}]' when calling
> method: [imICoreService::init]
> JavaScript error: file:///tbird-bin/dist/bin/components/imConversations.js,
> line 494: TypeError: this._prplConversations is undefined
> JavaScript error: file:///tbird-bin/dist/bin/components/imCore.js, line 293:
> NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error:
> "this._prplConversations is undefined" {file: "file:///tbird-bin/dist
> /bin/components/imConversations.js" line: 494}]'[JavaScript Error:
> "this._prplConversations is undefined" {file:
> "file:///tbird-bin/dist/bin/components/imConversations.js" line: 494}]' when
> calling method: [imIConversations
> Service::unInitConversations]

These also happen what starting TB normally and Services.contacts is undefined. I think that makes chat/IM completely broken right now.

Comment 43

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #42)
> I think that makes chat/IM completely broken right now.
Please remember that Daily runs with pref jsloader.shareGlobal set to 'false', so everything works. If you set that to 'true', then hell breaks loose.

Comment 44

[Jorg K (CEST = GMT+2)]

Comment on attachment 8911982 [details] [diff] [review]
1401528-gloda.patch v2 [landed: comment #60]

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

::: mailnews/db/gloda/modules/gloda.js
@@ +1819,2 @@
>        }
> +      this._attrProvidersByNoun[subjectType][aAttrDef.provider.providerName].push(aAttrDef);

How did this work before when we had:
[aAttrDef.provider] ?

Comment 45

[:aceman]

(In reply to Jorg K (GMT+2) from comment #43)
> Please remember that Daily runs with pref jsloader.shareGlobal set to
> 'false', so everything works. If you set that to 'true', then hell breaks
> loose.

Yes, I assume the possibility to set it to false is a temporary hack and will be removed by m-c after some time. There should also be some perf improvements with it set to true.

Comment 46

[:aceman]

(In reply to Jorg K (GMT+2) from comment #44)
> > +      this._attrProvidersByNoun[subjectType][aAttrDef.provider.providerName].push(aAttrDef);
> How did this work before when we had:
> [aAttrDef.provider] ?

I think asuth also confirms that this this._attrProvidersByNoun is not used anywhere. So it is just set at multiple places but never read. So even if it silently did nothing (due to the hidden error) it did no harm. Now the error is exposed, but fixing it also has no benefit to the algorithm, just silencing the error.

Comment 47

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8911949 [details] [diff] [review]
1401528-smtpd.patch [landed: comment #60]

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

This isn't the cleanest code, but I'm not too worried about it.

Comment 48

[Tooru Fujisawa [:arai]]

Comment on attachment 8911733 [details] [diff] [review]
1401528-textdecoder2.patch - NOT WORKING (see comment 31 and 32)

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

looks good

Comment 49

[Jorg K (CEST = GMT+2)]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

You put your r+ onto the wrong patch ;-( The |this.TextDecoder| doesn't work, see comment #31, comment #32 and comment #33.

Comment 50

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to :aceman from comment #35)
> There is an interesting error in calendar/test/unit/test_gdata_provider.js
> JavaScript strict warning:
> resource://gdata-provider/modules/gdataSession.jsm, line 517:
> ReferenceError: reference to undefined property "Array"
> TypeError: this.Array is undefined at
> resource://gdata-provider/modules/gdataSession.jsm:517
> 
> The code on this line is some obfuscated JS. Maybe I could decode it in some
> way but there is a big fat warning not to do it.
> Fallen can you look at that first?

The decoded code uses the Array global, and in that context |this| is the global object. I'm not getting this locally, can you file a new bug with more details if this is still an issue?

Comment 51

[Jorg K (CEST = GMT+2)]

You're not getting it locally since we set pref jsloader.shareGlobal to false (otherwise TB would fall apart in a screaming heap right now) :-(

We're here to fix all the issues that we see when setting jsloader.shareGlobal to true.

Comment 52

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: 1401528-gdata.patch

This fixes the gdata part, I went for the simple option. I was able to repro the MockRegistrar case at first, but no longer. If it still shows, you could try changing |this.MockRegistrar = ... | to |var MockRegistrar = ...| in that module.

Comment 53

[Jorg K (CEST = GMT+2)]

Comment on attachment 8917120 [details] [diff] [review]
1401528-gdata.patch

That's a patch, right? What's that obfuscated stuff?

Comment 54

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: 1401528-gdata.patch v2 [landed: comment #60]

It is a patch. Read the comment above it then you'll know :) Come to think of it though, it will fix the bustage but possibly break the provider. Here is an updated version that does not break the provider.

Comment 55

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8917165 [details] [diff] [review]
1401528-gdata.patch v2 [landed: comment #60]

I don't think it would be sensible to ask anyone to review this so I am going to rubberstamp it myself.

Comment 56

[Jorg K (CEST = GMT+2)]

(In reply to Philipp Kewisch [:Fallen] from comment #54)
> It is a patch. Read the comment above it then you'll know :)
I read the comment and didn't understand it. How would understanding what's going on make Google revoke anything? As far as I know, my thought process is not (yet) controlled by Google. Having code in the system that's hard to understand doesn't really improve maintainability. Besides, it's really easy to get my own copy of what exactly?

Comment 57

[Tooru Fujisawa [:arai]]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

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

Can you just use other name than TextDecoder?
looks like jsmime.js is the only consumer of the modified TextDecoder, and I think replacing the name there will solve the conflict, and we can just use "var" for new one.

Comment 58

[Jorg K (CEST = GMT+2)]

Thanks Tooru, I'll do a patch along those lines.

To get the bug moving again, I'll try and land the four r+ patches here first:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=14133ac94f12a03b5ee5b5abcbc5fe18a85d9288

Then we can turn our attention to JSMime again and then to the remaining failures, if any.

Comment 59

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #34)
> The last mozmill failure in mozmill\im\test-toolbar-buttons.js may be caused
> by this:
Please check bug 1407719 and attachment 8919909 [details] [diff] [review]. Sadly I had to back that out but it's along the lines of what we're looking for, I'm sure it can be made to work. See bug 1407719 comment #13 for the backout reason.

So after landing the four patches, JSMime and chat are all that's left :-) - We should really bring this to a close before treatment of globals in M-C changes and leaves us busted.

Comment 60

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4fd348dc8b0c
check of existence of softMargin and hardMargin in options to HeaderEmitter. r=jcranmer
https://hg.mozilla.org/comm-central/rev/c82304da4174
do not reference non-existent args[1] in fakeserver smtpd.js. r=jcranmer
https://hg.mozilla.org/comm-central/rev/5f5b8b76159a
do not access aAttrDef.special if not defined in gloda.js. r=asuth
https://hg.mozilla.org/comm-central/rev/2ae6e4e072d5
fix use of Array global in Gdata session. r=me

Comment 61

[Jorg K (CEST = GMT+2)]

(In reply to Tooru Fujisawa [:arai] from comment #57)
> Can you just use other name than TextDecoder?
> looks like jsmime.js is the only consumer of the modified TextDecoder, and I
> think replacing the name there will solve the conflict, and we can just use
> "var" for new one.
Thanks. What about, for example:
https://dxr.mozilla.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#1925
  charset = new TextDecoder(gMsgCompose.compFields.characterSet).encoding;

Does that get the stock-standard one or the modified one?

Why is FRG's suggestion so bad?
var {TextDecoder} = Components.utils.getGlobalForObject(
  Components.utils.import("resource://gre/modules/Services.jsm", {})
);

Comment 62

[Tooru Fujisawa [:arai]]

(In reply to Jorg K (GMT+2) from comment #61)
> (In reply to Tooru Fujisawa [:arai] from comment #57)
> > Can you just use other name than TextDecoder?
> > looks like jsmime.js is the only consumer of the modified TextDecoder, and I
> > think replacing the name there will solve the conflict, and we can just use
> > "var" for new one.
> Thanks. What about, for example:
> https://dxr.mozilla.org/comm-central/source/mail/components/compose/content/
> MsgComposeCommands.js#1925
>   charset = new TextDecoder(gMsgCompose.compFields.characterSet).encoding;
> 
> Does that get the stock-standard one or the modified one?

I don't see any reason it gets modified one.
the jsmime.jsm is modifying their own global's binding, and jsmime.js is loaded into the global, so it's using the same binding.
MsgComposeCommands.js has nothing to do with the global.


> Why is FRG's suggestion so bad?
> var {TextDecoder} = Components.utils.getGlobalForObject(
>   Components.utils.import("resource://gre/modules/Services.jsm", {})
> );

I thought you were looking for other option than that one.
if it works, that would be fine.
(I'm not able to test because I cannot build it right now)

Comment 63

[Jorg K (CEST = GMT+2)]

(In reply to Tooru Fujisawa [:arai] from comment #62)
> > Why is FRG's suggestion so bad?
> > var {TextDecoder} = Components.utils.getGlobalForObject(
> >   Components.utils.import("resource://gre/modules/Services.jsm", {})
> > );
> I thought you were looking for other option than that one.
> if it works, that would be fine.
It works fine, only Joshua did not seem to like it.

I'll try |var MimeTextDecoder| as you suggested in comment #57.

Comment 64

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-MimeTextDecoder.patch

Given that attachment 8911733 [details] [diff] [review] doesn't work and attachment 8911589 [details] [diff] [review] appears to be undesired, here's Tooru's suggestion. This one works, no errors and the real decoder is used.

Of course the dump and setting pref("jsloader.shareGlobal", true); is only for demonstration purposes.

I'd really like to bring this bug to a close with the JSMime part and the missing chat part.

Comment 65

[Tooru Fujisawa [:arai]]

Comment on attachment 8921389 [details] [diff] [review]
1401528-MimeTextDecoder.patch

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

the following file seems to be using jsmime.js (not jsm).
https://dxr.mozilla.org/comm-central/source/mailnews/mime/jsmime/test/head_xpcshell_glue.js

maybe it should also provide the same binding?
if providing same binding (maybe not necessarily re-implementing fake one, just var MimeTextDecoder = TextDecoder) works, this patch looks good.

::: mailnews/mime/src/jsmime.jsm
@@ +58,5 @@
>  function FallbackTextDecoder(charset, options) {
>    try {
>      return new RealTextDecoder(charset, options);
>    } catch (e) {
> +    dump("=== Using FakeTextDecoder\n");

this should be removed.

Comment 66

[Jorg K (CEST = GMT+2)]

(In reply to Tooru Fujisawa [:arai] from comment #65)
> the following file seems to be using jsmime.js (not jsm).
> https://dxr.mozilla.org/comm-central/source/mailnews/mime/jsmime/test/
> head_xpcshell_glue.js
> maybe it should also provide the same binding?
Hmm, nothing in that file uses a TextDecoder.
TextDecoder is only used very little in C-C
https://dxr.mozilla.org/comm-central/search?q=TextDecoder
and the uses are other covered in the patch or separate anyway like in MsgComposeCommands.js#1925.
Am I missing something?

> > +    dump("=== Using FakeTextDecoder\n");
> this should be removed.
Of course, I sometimes leave debug in the patch if the reviewer wants to try it, see comment #64 (quote):
  Of course the dump ... is only for demonstration purposes.

Comment 67

[Tooru Fujisawa [:arai]]

(In reply to Jorg K (GMT+2) from comment #66)
> (In reply to Tooru Fujisawa [:arai] from comment #65)
> > the following file seems to be using jsmime.js (not jsm).
> > https://dxr.mozilla.org/comm-central/source/mailnews/mime/jsmime/test/
> > head_xpcshell_glue.js
> > maybe it should also provide the same binding?
> Hmm, nothing in that file uses a TextDecoder.
> TextDecoder is only used very little in C-C
> https://dxr.mozilla.org/comm-central/search?q=TextDecoder
> and the uses are other covered in the patch or separate anyway like in
> MsgComposeCommands.js#1925.
> Am I missing something?

it's about the jsmime.js's assumption about the global binding.
now it expects there's MimeTextDecoder defined in global binding.
so, whatever uses jsmime.js should define it, just like jsmime.jsm does.

Comment 68

[:aceman]

(In reply to Jorg K (GMT+2) from comment #56)
> (In reply to Philipp Kewisch [:Fallen] from comment #54)
> > It is a patch. Read the comment above it then you'll know :)
> I read the comment and didn't understand it. How would understanding what's
> going on make Google revoke anything? As far as I know, my thought process
> is not (yet) controlled by Google. Having code in the system that's hard to
> understand doesn't really improve maintainability.

It may contain some "secret" app-specific token that Google gave out only for use by TB (or the gdata provider module).
This token should not be used by other apps so it is obfuscated a bit. Note the comment says that Google may revoke the access when you "use the information", not just read it.
It may be something similar to the OAuth2 brokenness.

> Besides, it's really easy
> to get my own copy of what exactly?

You may probably request your own token from Google. It will not be the same as the code we see in TB/Lightning, but will give you access to the Google calendar.

The ridiculous conditions are imposed by Google, but if we want the access, we need to carry the code. If Fallen can fix it, then we probably do not need to care much. Hopefully the code does not contain any other bugs/backdoors :)

Comment 69

[:aceman]

(In reply to Jorg K (GMT+2) from comment #59)
> So after landing the four patches, JSMime and chat are all that's left :-) -
> We should really bring this to a close before treatment of globals in M-C
> changes and leaves us busted.

Is the "processType" problem solved somewhere?

Comment 70

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to :aceman from comment #68)
> (In reply to Jorg K (GMT+2) from comment #56) 
> > Besides, it's really easy
> > to get my own copy of what exactly?
> 
> You may probably request your own token from Google. It will not be the same
> as the code we see in TB/Lightning, but will give you access to the Google
> calendar.
> 
> The ridiculous conditions are imposed by Google, but if we want the access,
> we need to carry the code. If Fallen can fix it, then we probably do not
> need to care much. Hopefully the code does not contain any other
> bugs/backdoors :)

I think it is reasonable that this piece of code will only be maintained by me. It is not impossible to decode that if you really want to know what it means. This is how OAuth works, the protocol was rather made for websites authenticating where the keys can remain secret. It was not very much made for client applications, especially not for open source applications. Keeping this obfuscated is a requirement that has been discussed with folks from Google.

Comment 71

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #69)
> Is the "processType" problem solved somewhere?
Not to my knowledge. Let's ask the assignee, oops there isn't one, so the guy who wrote (n-2) of the patches should know ;-)

Comment 72

[Jorg K (CEST = GMT+2)]

Thanks for the try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3727f80b3ecc894a7c348275b8da927b61df957f
Looks quite terrible :-(

Comment 73

[:aceman]

I couldn't spot MockRegistrar errors in the logs anymore. Can you?

Comment 74

[:aceman]

Comment on attachment 8921389 [details] [diff] [review]
1401528-MimeTextDecoder.patch

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

Sorry, the try run with the patch shows the patch is not fully working. There are no problems with TextDecoder now, but with MimeTextDecoder:
"ReferenceError: MimeTextDecoder is not defined"
See the xpcshell result at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3727f80b3ecc894a7c348275b8da927b61df957f .

Comment 75

[Jorg K (CEST = GMT+2)]

(In reply to Tooru Fujisawa [:arai] from comment #65)
> the following file seems to be using jsmime.js (not jsm).
> https://dxr.mozilla.org/comm-central/source/mailnews/mime/jsmime/test/
> head_xpcshell_glue.js

Hmm, looking at
https://archive.mozilla.org/pub/thunderbird/try-builds/[e-mail]-3727f80b3ecc894a7c348275b8da927b61df957f/try-comm-central-linux64/try-comm-central_ubuntu64_vm_test-xpcshell-bm53-tests1-linux64-build6.txt.gz
I see what Tooru-san predicted:

INFO -  Unexpected exception ReferenceError: MimeTextDecoder is not defined at /builds/slave/test/build/tests/xpcshell/tests/mailnews/mime/jsmime/test/head_xpcshell_glue.js -> resource:///modules/jsmime/jsmime.js:585

(In reply to Tooru Fujisawa [:arai] from comment #67)
> it's about the jsmime.js's assumption about the global binding.
> now it expects there's MimeTextDecoder defined in global binding.
> so, whatever uses jsmime.js should define it, just like jsmime.jsm does.

Sadly my JS skills are rather mediocre, so I don't understand "global binding". Tooru-san, could you elaborate a little or give a reference where I could read up on it. What needs to be done in head_xpcshell_glue.js exactly?

Oh, perhaps |var MimeTextDecoder;| so the variable is declared for jsmime.js to use. But we want the variable from the jsmime.jsm module, or not? So how do we do that?

Or Aceman, do you know?

Comment 76

[Tooru Fujisawa [:arai]]

currently, at least in 2 cases, jsmime.js is loaded with Services.scriptloader.loadSubScript.
  * jsmime.jsm
  * head_xpcshell_glue.js

In jsmime.jsm's case, jsmime.jsm defines global variable MimeTextDecoder on jsmime.jsm's side,
and jsmime.js uses the variable.

So, on the other side, head_xpcshell_glue.js's case, it should define the same variable on xpcshell_glue.js's side,
otherwise jsmime.js cannot use the variable.

then, previously jsmime.jsm was modifying TextDecoder binding (with fake/real ones), and head_xpcshell_glue.js was not.
so, in head_xpcshell_glue.js's case, you just need to bind MimeTextDecoder to TextDecoder, just before loading it.
  var MimeTextDecoder = TextDecoder;

Comment 77

[Jorg K (CEST = GMT+2)]

Hey, thanks, that's crystal clear now ;-)

Comment 78

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-MimeTextDecoder.patch (v2)

OK, adding one line to xpcshell_glue.js as per Tooru-san's specification. Let's see how this will work out:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=300404ae66732a53a97376ca9e7716d769a8b0d2

I had to include the M-C patch for bug 1412497 and we know that test_pop3PasswordFailure.js still fails due to that bug.

Carrying forward Tooru-san's f+ ;-)

Comment 79

[Jorg K (CEST = GMT+2)]

Comment on attachment 8923209 [details] [diff] [review]
1401528-MimeTextDecoder.patch (v2)

Hmm, that still gave a bazillion mostly calendar test failures:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=140562041
https://archive.mozilla.org/pub/thunderbird/try-builds/[e-mail]-300404ae66732a53a97376ca9e7716d769a8b0d2/try-comm-central-macosx64/try-comm-central_yosemite_r7_test-xpcshell-bm107-tests1-macosx-build1.txt.gz
but not JSMime related.

I get 92 of these:
JavaScript error: resource://calendar/modules/calUtils.jsm, line 1045: NS_ERROR_XPC_GS_RETURNED_FAILURE: Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]
which might explain the many calendar failures.

There are the already known chat failures:
TypeError: this.__defineGetter__ is not a function at resource:///modules/xmpp-session.jsm:24

but there are also a few miscellaneous mailnews failures.

So what's the way forward with this bug?

Adding r?jcranmer here since I've discussed this with him on IRC last night.

Comment 80

[Jorg K (CEST = GMT+2)]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

Joshua, please decide whether you want this patch or the other one. Your suggestion of |this.TextDecoder| doesn't work.

Comment 81

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to Jorg K (GMT+2) from comment #79)

> I get 92 of these:
> JavaScript error: resource://calendar/modules/calUtils.jsm, line 1045:
> NS_ERROR_XPC_GS_RETURNED_FAILURE: Component returned failure code:
> 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]
> which might explain the many calendar failures.
This is not very helpful yet, we'd have to know the call stack to see which service is failing, then see what the failure in the specific service is. This is probably in a commonly included file so that a simple fix will fix them all.

> 
> There are the already known chat failures:
> TypeError: this.__defineGetter__ is not a function at
> resource:///modules/xmpp-session.jsm:24
I supposed they removed __defineGetter__ in favor of Object.defineProperty().

Calendar also uses __defineGetter__ iirc, maybe fixing that would also fix the getService failures.

Comment 82

[Jorg K (CEST = GMT+2)]

(In reply to Philipp Kewisch [:Fallen] from comment #81)
> This is not very helpful yet, ...
I hope you're not talking to me. I'm not the assignee here, Aceman bravely assigned himself. I fixed the bustage in comment #3 and now I'm interested to see this bug being closed. My contribution is only the JS Mime bit, which really is from FRG from bug 1401897 comment #14.

Comment 83

[Jorg K (CEST = GMT+2)]

Sorry, I just saw: Aceman is not assigned, but anyway, he's sent most patches here.

Comment 84

[:aceman]

I just fixed some of the problems I could.
Feel free to hunt the processType problem.
I assume the chat/IM problems are to be fixed in bug 1407719.

Comment 85

[:aceman]

Comment on attachment 8923209 [details] [diff] [review]
1401528-MimeTextDecoder.patch (v2)

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

::: mailnews/mime/jsmime/test/head_xpcshell_glue.js
@@ +44,5 @@
>             .then(contents => callback(undefined, contents), callback);
>    },
>  };
>  requireCache.set("fs", fs);
> +var MimeTextDecoder = TextDecoder;

Why can't we import jsmime.jsm here?
Or make jsmime.js import jsmime.jsm ?
You said, we just need to ask jcranmer :)

Comment 86

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #84)
> I just fixed some of the problems I could.
Please continue hunting.

Comment 87

[Jorg K (CEST = GMT+2)]

As per comment #0 this first failed on 20th Sept 2017 here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=4ff3727aec0f13b8a9912b7fadab5fee26249302&selectedJob=132204570
The 55 failing Xpcshell tests are listed in comment #0.

In my try run
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=300404ae66732a53a97376ca9e7716d769a8b0d2&selectedJob=140562041
I'm seeing 66 calendar failures, so by simple math, there are some that didn't fail before. Let me just quote some not listed in comment #0:
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_bug356207.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_calmgr.js | xpcshell return code: 0
So it looks to me that some code got added that is aggravating the failures.

In a local build with the patch
mozilla/mach xpcshell-test calendar/test/unit/test_bug356207.js
appears to be failing with

 0:02.39 TEST_STATUS: Thread-1 really_run_test FAIL NS_ERROR_XPC_GS_RETURNED_FAILURE: Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]
_service/<@resource://calendar/modules/calUtils.jsm:1045:16
parseString@resource://calendar/modules/calUtils.jsm -> file:///c:/mozilla-source/comm-central/obj-i686-pc-mingw32/dist/bin/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/calendar-js/calIcsParser.js:145:39
createEventFromIcalString@c:/mozilla-source/comm-central/obj-i686-pc-mingw32/_tests/xpcshell/calendar/test/unit/head_consts.js:49:9
really_run_test@c:/mozilla-source/comm-central/obj-i686-pc-mingw32/_tests/xpcshell/calendar/test/unit/test_bug356207.js:36:17
run@c:\mozilla-source\comm-central\mozilla\testing\xpcshell\head.js:701:9
_do_main@c:\mozilla-source\comm-central\mozilla\testing\xpcshell\head.js:223:3
_execute_test@c:\mozilla-source\comm-central\mozilla\testing\xpcshell\head.js:544:5
@-e:1:1

 0:02.39 LOG: Thread-1 INFO exiting test

Just to be sure that my MIME changes haven't changed anything, I repeated the test with patch "1401528-textdecoder.patch" which is known to work and get the same failure.

So I'd appreciate if the calendar people could look into those test failures before M-C remove the "jsloader.shareGlobal" preference.

We have enough non-calender failures to look at :-(

Comment 88

[Jorg K (CEST = GMT+2)]

Looking at the processType error in
nsLoginManager.js, line 98
  if (Services.appinfo.processType ===
AsyncShutdown.jsm, line 71
  const isContent = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime).processType ==
    Ci.nsIXULRuntime.PROCESS_TYPE_CONTENT;
osfile_async_front.jsm, line 1430
  const isContent = Components.classes["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime).processType ==
    Ci.nsIXULRuntime.PROCESS_TYPE_CONTENT;

I have no idea why this fails since it appears to come from
xpcom/system/nsIXULRuntime.idl
  readonly attribute unsigned long processType;

I've talked to a few people on #developers. The story is this:
21:58:53 - John-Galt: jorgk: We tend to override the XULRuntime service in tests with incomplete JS stubs
22:01:04 - jorgk: John-Galt: And those stubs don't give processType?
22:02:15 - John-Galt: jorgk: In general, no. Though we should probably fix that.
22:03:02 - jorgk: So why would tweaking "jsloader.shareGlobal" from false to true get us into the stub that doesn't do what is required?
22:06:53 - mccr8: jorgk: are these XPCShell tests?
22:06:58 - jorgk: yes
22:07:34 - jorgk: BTW, is that "jsloader.shareGlobal" pref likely to disappear one day soon?
22:07:45 - mccr8: jorgk: so the basic problem is that some tests override this runtime thing. If each JSM has its own compartment, then doing the overriding only affects that one JSM that did the override. But with the shared compartment, it overrides it for everything.
22:08:06 - mccr8: jorgk: we don't any plans to remove it, but it is totally untested on Firefox, so it could break.
22:13:44 - jorgk: mccr8: what is untested? The true behaviour or the false behaviour? FF runs on true, no?
22:14:09 - mccr8: jorgk: the false behavior is untested
22:14:42 - mccr8: jorgk: the problem you are seeing sounds familiar to me. I'm trying to read what I did to fix it...
22:16:24 - jorgk: mccr8: So if the false behaviour is untested and my disappear soon, we should quickly fix everything so we can run with true, right?
22:17:19 - mccr8: jorgk: I doubt it will disappear soon, as it isn't a big deal to leave it as is. The main problem would be Firefox .jsms implicitly starting to rely on it and those breaking.
22:17:36 - mccr8: jorgk: I don't know how much Thunderbird uses in terms of Firefox jsms
22:18:35 - mccr8: jorgk: I think I saw tons of errors locally similar to "ReferenceError: reference to undefined property "processType"" that weren't actually causing tests to fail. So I'd make sure those are actually causing issues.
22:18:58 - jorgk: mccr8: Well, in the tests that fail, we're using osfile_async_front.jsm and AsyncShutdown.jsm
22:19:25 - mccr8: jorgk: I think this is the thing I added to fix the app info issue I was seeing: http://searchfox.org/mozilla-central/source/testing/modules/AppInfo.jsm#137
22:24:02 - jorgk: mccr8: Thanks. Funny that like you pointed me to: Cc.initialize(Cc[cid], cid);
22:24:48 - jorgk: mccr8: We also have many failures in our Calendar part on this line: return Components.classes[cid].getService(iid);
22:25:47 - jorgk: mccr8: That gets us: FAIL NS_ERROR_XPC_GS_RETURNED_FAILURE: Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]
22:26:12 - mccr8: jorgk: the basic problem is that the first time you do Cc[cid] in a compartment, it computes what the value should be, then it saves it forever.
22:27:12 - mccr8: jorgk: so if you used to do something like: load foo.jsm (which does Cc[cid]), then load bar.jsm (which binds something else to cid), then load bar2.jsm (which looks at Cc[cid]) then you'll get something different in bar2.jsm
22:27:24 - mccr8: now you'll get whatever foo.jsm got, but before you'd get the new thing added by bar.jsm
22:27:50 - mccr8: This comes up in XPCShell because AppInfo.jsm gets mapped to some app info thing.
22:28:52 - jorgk: mccr8: And also apparently in some JS Calendar code.
22:29:49 - mccr8: jorgk: yeah, so you'll need to look for something like " registrar.registerFactory(id, "XULAppInfo", cid, factory);" and then put that same little line there.
22:30:10 - jorgk: mccr8: Sadly I'm not much of a JS guy, so I'll copy the chat into the bug and let other people worry about it.
22:30:28 - mccr8: jorgk: generally speaking, you can look through the patches in bug 1186409 and the many blocking bugs for changes to JS files for things I had to do to work around problems from sharing.
22:30:30 - jorgk: mccr8: Thanks for now, we have a lead ;-)
22:30:35 - RyanVM is now known as RyanVM|brb.
22:30:45 - mccr8: jorgk: alright. Yeah this is mostly a matter of debugging JS code.

So Philipp and Aceman, would you like to continue here with this information?

Comment 89

[Jorg K (CEST = GMT+2)]

(In reply to Philipp Kewisch [:Fallen] from comment #81)
> I supposed they removed __defineGetter__ in favor of Object.defineProperty().
> Calendar also uses __defineGetter__ iirc, maybe fixing that would also fix
> the getService failures.
Yes, bingo: Bug 1363215 blocking bug 1186409.

Comment 90

[alta88]

(In reply to Philipp Kewisch [:Fallen]  from comment #50)
> (In reply to :aceman from comment #35)
> > There is an interesting error in calendar/test/unit/test_gdata_provider.js
> > JavaScript strict warning:
> > resource://gdata-provider/modules/gdataSession.jsm, line 517:
> > ReferenceError: reference to undefined property "Array"
> > TypeError: this.Array is undefined at
> > resource://gdata-provider/modules/gdataSession.jsm:517
> > 
> > The code on this line is some obfuscated JS. Maybe I could decode it in some
> > way but there is a big fat warning not to do it.
> > Fallen can you look at that first?
> 
> The decoded code uses the Array global, and in that context |this| is the
> global object. I'm not getting this locally, can you file a new bug with
> more details if this is still an issue?

There is a similar obfuscated app secret in nsBox.js which fails in the same way.
https://dxr.mozilla.org/comm-central/rev/306fff0f8a9d1fe8b7eeafd86c68166d78288767/mail/components/cloudfile/nsBox.js#810

Comment 92

[:aceman]

(In reply to Jorg K (GMT+1) [currently bustage-fix only, no NI? or r?] from comment #89)
> (In reply to Philipp Kewisch [:Fallen] from comment #81)
> > I supposed they removed __defineGetter__ in favor of Object.defineProperty().
> > Calendar also uses __defineGetter__ iirc, maybe fixing that would also fix
> > the getService failures.
> Yes, bingo: Bug 1363215 blocking bug 1186409.

The bug seems to be fixed, so now we need to port this to IM, that is still throwing this (comment 42).
I can try to do what https://hg.mozilla.org/mozilla-central/rev/eab436c452e9 does, and always set configurable=true and enumerable=true (as I don't kno what they do).

But we have some of those in /mail and /mailnews too and even in /mozilla. It seems the conversion only needs to be done in shared modules (jsm) for now.

Comment 93

[:aceman]

Attachment: 1401528-chat.patch [landed comment #110]

This fixes all the chat xpcshell tests for me and also local TB startup no longer spews the errors about __defineGetter__.

Comment 94

[Jorg K (CEST = GMT+2)]

Try run with attachment 8923209 [details] [diff] [review], attachment 8931968 [details] [diff] [review] and the M-C patch from bug 1420758:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c6b948388e5a41d4fcc52e73c03274f15a097df4
on request of a single gentleman who doesn't want the Sword of Damocles to cut through the thin silken thread by which we're hanging (how's that for a nice mix of flowery language). Thank you!

BTW, looking at NI?Fallen and comment #87, there's also a heap of calendar failures, no?

Comment 95

[Jorg K (CEST = GMT+2)]

Comment on attachment 8931968 [details] [diff] [review]
1401528-chat.patch [landed comment #110]

This patch looks a lot like attachment 8919909 [details] [diff] [review] from bug 1407719. Looks like both try to fix the same problem.

Comment 96

[:aceman]

Yes, it looks the same. IB probably runs with pref("jsloader.shareGlobal", true). But is IB dead now?

Comment 97

[Jorg K (CEST = GMT+2)]

Apparently so. The patch got backed out, BTW, bug 1407719 comment #14.

Comment 98

[Jorg K (CEST = GMT+2)]

Sorry, I should have done and "opt" try, here we go again:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f05c9145e423130e35265f77a99e07975f8aa0d6

Comment 99

[:aceman]

Thanks.
The results are much better now.

The remaining failures (not counting calendar) seem to be this:
###!!! ASSERTION: attempted to open a new window with no WindowCreator: 'mWindowCreator', file /builds/slave/tb-try-c-cen-l64-d-00000000000/build/mozilla/toolkit/components/windowwatcher/nsWindowWatcher.cpp, line 937
nsWindowWatcher::OpenWindow [toolkit/components/windowwatcher/nsWindowWatcher.cpp:361]

I'd guess defining the dummy window in alertTestUtils.js needs some change.

Comment 101

[Jorg K (CEST = GMT+2)]

Does chat still work with the chat patch? See bug 1407719 comment #13. The patch over there broke chat, hence the backout.

Comment 102

[Patrick Cloke [:clokep]]

Comment on attachment 8931968 [details] [diff] [review]
1401528-chat.patch [landed comment #110]

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

On the surface this looks fine. I'd like confirmation that this doesn't break chat completely though (per bug 1407719 comment 13).

Comment 103

[:aceman]

I'm not able to check if chat is still working globally. I tried IRC and it works. I can't say if any of the touched code (imContacts, imSmileys) is run in the IRC codepath. I hope clokep as a chat peer can check this better.

Comment 104

[:aceman]

Paenglab, do you know what you did to get bug 1407719 comment 13 ?

Comment 105

[Jorg K (CEST = GMT+2)]

As far as I know Richard, he uses IRC and Twitter. The problem in bug 1407719 comment 13 was:
  Error in the console: TypeError: property "gTheme" is non-configurable and can't be deleted  imSmileys.jsm:20:5
since the original patch didn't make the property configurable:
https://hg.mozilla.org/comm-central/rev/4f4b4c62ffd1#l2.16
This has now been improved in Aceman's patch, so everything should be working.

Comment 106

[Richard Marti (:Paenglab)]

(In reply to :aceman from comment #104)
> Paenglab, do you know what you did to get bug 1407719 comment 13 ?

Only logged in to IRC or Twitter. Then where are no messages shown, completely blank. When you see messages like the channel summary then it seems to work.

Comment 107

[:aceman]

(In reply to Jorg K (GMT+1) from comment #105)
> As far as I know Richard, he uses IRC and Twitter. The problem in bug
> 1407719 comment 13 was:
>   Error in the console: TypeError: property "gTheme" is non-configurable and
> can't be deleted  imSmileys.jsm:20:5
> since the original patch didn't make the property configurable:
> https://hg.mozilla.org/comm-central/rev/4f4b4c62ffd1#l2.16
> This has now been improved in Aceman's patch, so everything should be
> working.

Yes, I also added the configurable=true and enumerable=true as the m-c bug did.

I tested IRC and it does work without errors in console (nothing about the problem with gTheme). I went to the channel and everything appeared OK.

(In reply to Patrick Cloke [:clokep] from comment #102)
> On the surface this looks fine. I'd like confirmation that this doesn't
> break chat completely though (per bug 1407719 comment 13).

We'd expect you can ensure the confirmation. Chat is broken completely without the patch. With the patch at least TB is not broken completely. I'm sure you can test the other protocols better than us.
I've run chat tests locally and also try run is at https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f05c9145e423130e35265f77a99e07975f8aa0d6

Comment 108

[Florian Quèze [:florian]]

Comment on attachment 8931968 [details] [diff] [review]
1401528-chat.patch [landed comment #110]

Looks good to me by code inspection. Comment 107 seems good enough as a verification that this won't cause the issue in bug 1407719 comment 13. Displaying any chat message will definitely initialize the imSmileys.jsm code path. Thanks for fixing these failures!

Comment 109

[Jorg K (CEST = GMT+2)]

I'll get the chat piece landed which is also the fix for bug 1407719.

Comment 110

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1f76939382a1
replace __defineGetter__ on top-level objects in chat. r=florian

Comment 111

[:aceman]

Comment on attachment 8923209 [details] [diff] [review]
1401528-MimeTextDecoder.patch (v2)

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

I don't like this due to the definition of MimeTextDecoder in the test file. We shouldn't have to define something that is already an internal detail of the core code.

Comment 112

[Jorg K (CEST = GMT+2)]

Just for the record, apart from heaps of calendar failures, we have these password related failures:
mailnews/compose/test/unit/test_bug155172.js
mailnews/compose/test/unit/test_smtpPasswordFailure2.js
mailnews/compose/test/unit/test_smtpPasswordFailure3.js
mailnews/imap/test/unit/test_imapPasswordFailure.js
mailnews/local/test/unit/test_pop3PasswordFailure.js
mailnews/local/test/unit/test_pop3PasswordFailure2.js
mailnews/local/test/unit/test_pop3PasswordFailure3.js

mailnews/compose/test/unit/test_smtpPasswordFailure1.js doesn't fail.

The failure is always a crash like this:
"authorize-id: --, username: -testsmtp-, password: -smtptest-"
"Enter new password"
"[5324, Main Thread] WARNING: NS_ENSURE_TRUE(m_callbacks) failed: file c:/mozilla-source/comm-central/mailnews/compose/src/nsSmtpUrl.cpp, line 817"
"[5324, Main Thread] ###!!! ASSERTION: attempted to open a new window with no WindowCreator: 'mWindowCreator', file c:/mozilla-source/comm-central/mozilla/toolkit/components/windowwatcher/nsWindowWatcher.cpp, line 937"
"#01: nsWindowWatcher::OpenWindowInternal (c:\\mozilla-source\\comm-central\\mozilla\\toolkit\\components\\windowwatcher\\nswindowwatcher.cpp:936)"
"#02: nsWindowWatcher::OpenWindow (c:\\mozilla-source\\comm-central\\mozilla\\toolkit\\components\\windowwatcher\\nswindowwatcher.cpp:355)"
"#03: _NS_InvokeByIndex (c:\\mozilla-source\\comm-central\\mozilla\\xpcom\\reflect\\xptcall\\md\\win32\\xptcinvoke_asm_x86_msvc.asm:57)"
...
"#45: nsSmtpServer::GetPasswordWithUI (c:\\mozilla-source\\comm-central\\mailnews\\compose\\src\\nssmtpserver.cpp:447)"
"#46: nsSmtpProtocol::PromptForPassword (c:\\mozilla-source\\comm-central\\mailnews\\compose\\src\\nssmtpprotocol.cpp:2237)"
"#47: nsSmtpProtocol::GetPassword (c:\\mozilla-source\\comm-central\\mailnews\\compose\\src\\nssmtpprotocol.cpp:2195)"
"#48: nsSmtpProtocol::AuthLoginStep1 (c:\\mozilla-source\\comm-central\\mailnews\\compose\\src\\nssmtpprotocol.cpp:1465)"
"#49: nsSmtpProtocol::ProcessProtocolState (c:\\mozilla-source\\comm-central\\mailnews\\compose\\src\\nssmtpprotocol.cpp:2056)"
"#50: nsMsgProtocol::OnDataAvailable (c:\\mozilla-source\\comm-central\\mailnews\\base\\util\\nsmsgprotocol.cpp:302)"

Comment 113

[:aceman]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

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

This seems to work, it is also what m-c does, so let's go with this unless a better solution is found in time.
I agree with arai that we should change the name of the final variable to MimeTextDecoder if possible. If it may not contain the standard m-c decoder, let's not call it the same.

Comment 114

[Jorg K (CEST = GMT+2)]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

This patch works and you can see that the "real" decoder is used. The inspiration comes from the references given in comment #8.

Comment 115

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-mock.patch [landed comment #137]

This fixes the password tests.

Clearly, looking at comment #112 we can see that something has gone wrong with the mocked service. I don't understand why we have two mocking approaches in the system, M-C's MockRegistrar and C-C's MockFactory, which has about the same come as the M-C version.

Simply switching to the M-C version fixes the problem. Maybe that can 1) be a hint of how to fix C-C's version be comparing it to the M-C version or 2) be the first step in tossing (Australian slang for "throwing away") the C-C version.

C-C's MockFactory is used elsewhere, so it's a little hard to see why it doesn't work here but works under other circumstances.

Maybe Aceman and Tooru-ran can look at this further.

Comment 116

[Jorg K (CEST = GMT+2)]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=90c2966bc4fb2f1aba60b64e8be2a66eecba58b8

Comment 117

[Tooru Fujisawa [:arai]]

what's "other circumstances" ?
all other consumers of MockFactory in c-c?

Comment 118

[Jorg K (CEST = GMT+2)]

In the try run from comment #116 we only have the calendar failures left, we'd need to look into them.

Does anyone understand why switching to M-C's MockRegistrar fixes the problem?

(In reply to Tooru Fujisawa [:arai] from comment #117)
> what's "other circumstances" ?
> all other consumers of MockFactory in c-c?
Yes, heaps of them unless DXR is lying to me:
https://dxr.mozilla.org/comm-central/sMockFactory earch?q=MockFactory&redirect=false
At least in
mail/base/test/unit/test_alertHook.js
mailnews/base/test/unit/test_compactFailure.js
mailnews/import/test/unit/resources/mock_windows_reg_factory.js
mailnews/test/resources/alertTestUtils.js - changed by the patch proposed here.

Comment 119

[Jorg K (CEST = GMT+2)]

Or maybe I misunderstood. Yes, all those other consumers seem to be working unless I'm missing something.

Comment 120

[:aceman]

Comment on attachment 8933815 [details] [diff] [review]
1401528-mock.patch [landed comment #137]

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

Interesting, this works for me too.
Notice there was also some MockRegistrar problem in calendar I reported, but it is no longer seen.

Comment 121

[Jorg K (CEST = GMT+2)]

So, do we just land the "mock" patch and move on, or do we try to understand what's going on? And do we use M-C's MockRegistrar for the the others C-C consumers of MockFactory as well?

Comment 122

[:aceman]

I difference I see in the c-c version and m-c version is that c-c one does not have a call to this.registrar.unregisterFactory(originalCID, originalFactory); inside register().

Other than that the Mock code seems very similar, the m-c one seems more modern. I think we could try to switch to the m-c one so that we get future improvements in it.

Comment 123

[:aceman]

(In reply to :aceman from comment #122)
> I difference I see in the c-c version and m-c version is that c-c one does
> not have a call to this.registrar.unregisterFactory(originalCID,
> originalFactory); inside register().

I'd think that when the objects are all in the same space properly registering and unregistering the objects becomes more critical.

Also I noticed in m-c version the new mock object is registered under the same originalCID as the original object (which is first unregistered). In the c-c version the mock is registered under a new UUID (possibly random). Making this change in c-c made the test_smtpPasswordFailure2.js test pass for me.

Comment 124

[:aceman]

(In reply to Jorg K (GMT+1) from comment #121)
> So, do we just land the "mock" patch and move on, or do we try to understand
> what's going on?

I have described where the difference lies. Maybe arai would know why it matters.

> And do we use M-C's MockRegistrar for the the others C-C
> consumers of MockFactory as well?

I try this.

Comment 125

[:aceman]

Attachment: 1401528-mockRegistrar.patch

Yes, the replacement works for me locally, even with the classic way of importing the module (no const {MockRegistrar} = Components.utils.import()). Does anybody know which way is preferred?

It also works whether jsloader.shareGlobal is true or false.

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

Comment 126

[Jorg K (CEST = GMT+2)]

Aceman, I'll leave it with you, at least we're on the right track here.

Comment 127

[:aceman]

(In reply to Jorg K (GMT+1) from comment #115)
> Simply switching to the M-C version fixes the problem. Maybe that can 1) be
> a hint of how to fix C-C's version be comparing it to the M-C version or 2)
> be the first step in tossing (Australian slang for "throwing away") the C-C
> version.

Thanks for looking at this and finding this out!

Comment 128

[:aceman]

Comment on attachment 8911589 [details] [diff] [review]
1401528-textdecoder.patch

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

With this patch, can we be sure that head_xpcshell_glue.js (and then jsmime.js) actually uses the replaced TextDecoder, when it does not import jsmime.jsm?
Please try changing the name of the variable (as requested) and also in jsmime.js and see what happens.

Comment 129

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-mock.patch, merged with aceman's

I merged the two patches with joint authorship and review. I don't think we need to spread the same idea over two patches. Here's a try with shared globals at false. Let's land this to fix the pen-ultimate problem.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3c490017bd36750d9c5d0b7e156f743f1e3e0938

Comment 130

[Jorg K (CEST = GMT+2)]

Comment on attachment 8923209 [details] [diff] [review]
1401528-MimeTextDecoder.patch (v2)

Not wanted, so let's obsolete it for less confusion.

Comment 131

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #128)
> Please try changing the name of the variable (as requested) and also in
> jsmime.js and see what happens.
Sorry, I don't understand the comment. The "change the variable" patch is attachment 8923209 [details] [diff] [review] which you don't like.

As I understand
var {TextDecoder} = Components.utils.getGlobalForObject(
  Components.utils.import("resource://gre/modules/Services.jsm", {}));
tells the module that 'TextDecoder' is the internal text decoder object, so that
TextDecoder = FallbackTextDecoder;
will not yield an "undefined" error but will instead overwrite that global object.

So if you require further experiments, please do them yourself since I wouldn't know what to do. I have r+ ;-)

Comment 132

[:aceman]

Comment on attachment 8933914 [details] [diff] [review]
1401528-mock.patch, merged with aceman's

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

Thanks, this seems to still work.

I'd still like to hear from arai which method of import is the preferred one.

Comment 133

[Jorg K (CEST = GMT+2)]

Comment on attachment 8933914 [details] [diff] [review]
1401528-mock.patch, merged with aceman's

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

::: mailnews/test/resources/alertTestUtils.js
@@ +30,5 @@
>   */
>  
>  Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> +const {MockRegistrar} =
> +  Components.utils.import("resource://testing-common/MockRegistrar.jsm", {});

Aceman is wondering about this hunk. I copied that from
https://searchfox.org/mozilla-central/source/browser/extensions/formautofill/test/unit/test_masterPassword.js#6
Since we test passwords, I thought his is just appropriate ;-)

Comment 134

[Tooru Fujisawa [:arai]]

Comment on attachment 8933914 [details] [diff] [review]
1401528-mock.patch, merged with aceman's

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

in most case they behave in the same way (not completely equal, but you won't see any notable difference),
so I think it's fine to just leave them as is, or choose whichever you like.

Comment 135

[Jorg K (CEST = GMT+2)]

Comment on attachment 8933914 [details] [diff] [review]
1401528-mock.patch, merged with aceman's

Gentlemen, bad news:
This patch affects various tests, in particular:
mail/base/test/unit/test_alertHook.js
That test is currently not run at all, so we can't see whether the changes here break it even further.

mailnews/import/test/unit/resources/mock_windows_reg_factory.js
used by
mailnews/import/test/unit/test_winmail.js and
mailnews/import/test/unit/test_outlook_settings.js

Both tests now fail with:
"CONSOLE_MESSAGE: (error) [JavaScript Error: "TypeError: this._registryData is undefined" {file: "resources/mock_windows_reg_factory.js" line: 13}]"
"CONSOLE_MESSAGE: (error) [JavaScript Error: "TypeError: this._registryData is undefined" {file: "resources/mock_windows_reg_factory.js" line: 13}]"

mailnews/base/test/unit/test_compactFailure.js. This fails for me locally with:
"JavaScript error: c:/mozilla-source/comm-central/obj-i686-pc-mingw32/_tests/xpcshell/mailnews/base/test/unit/test_compactFailure.js, line 126: uncaught exception: 2152857606"
"Ignoring XPCOM error: NS_ERROR_FILE_TARGET_DOES_NOT_EXIST"
"CONSOLE_MESSAGE: (error) [JavaScript Error: "uncaught exception: 2152857606" {file: "c:/mozilla-source/comm-central/obj-i686-pc-mingw32/_tests/xpcshell/mailnews/base/test/unit/test_compactFailure.js" line: 126}]"

So whilst the MockFactory replacement is a noble cause, it distracts from the main aim of this bug. We should move it to another bug and focus on the global issues here.

Comment 136

[Jorg K (CEST = GMT+2)]

Comment on attachment 8933815 [details] [diff] [review]
1401528-mock.patch [landed comment #137]

Let's revive this patch and get it landed.

Comment 137

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8209695b2056
fix password failure xpcshell tests by switching from C-C's MockFactory to M-C's MockRegistrar. r=aceman

Comment 138

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-textdecoder.patch (v1b) [landed comment #139]

Removed setting of preference and removed debug. Ready for landing.

Comment 139

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cea2dc7d3190
Use global scope for TextDecoder. r=jorgk,aceman

Comment 140

[Jorg K (CEST = GMT+2)]

Comment on attachment 8933971 [details] [diff] [review]
1401528-textdecoder.patch (v1b) [landed comment #139]

This bug is finally coming to an end. When bug 1422547 gets merged into M-C and we get review for
https://hg.mozilla.org/try-comm-central/rev/3a626f6ef2001392337bc5946e5d7581b0a421ab
we'll finally be done here after about three month (and just before Christmas) - \o/

Comment 141

[Jorg K (CEST = GMT+2)]

Attachment: 1401528-backout-temporay-fix.patch [landed comment #164]

Comment 142

[Jorg K (CEST = GMT+2)]

Since we're nearing the end here and Aceman did most the work, it's fair to give him the bug.

Comment 143

[:aceman]

Attachment: 1401528-MimeTextDecoder.patch

You landed the textdecoder patch without renaming the variable.
So this is the renaming. It is the same as your attachment 8923209 [details] [diff] [review], just the test includes jsmime.jsm (which includes jsmime.js) instead of jsmime.js. So then no duplicate definition of MimeTextDecoder is needed, which I objected to.

This renaming would make me feel better about which decoder is actually used, because in jsmime.jsm jsmime.js (which uses the decoder) is loaded sooner than we redefine the decoder at the end of the file.

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

Comment 144

[:aceman]

Attachment: 1401528-calendar.patch [landed comment #159]

This is the patch for the calendar breakage.

It seems calendar loads the services from libical and in calbackendLoader.js if the pref for icaljs is set, it unregisters them and loads them again from icaljs, under the same contractIDs, which is OK, but the preceding call to CC[contractId] seems to 'freeze' the previous versions (maybe the classIDs) and even if they are replaced by new ones from icaljs, the new versions are not picked up. See comment 88.

So this changes the unregistering to not need to reference CC[contractId]. It is the line 'let classobj = Components.classes[contractId];' that makes the difference. Getting the factory works whether Cm.getClassObject() is called on classobj or classId.

This fixes all the calendar tests EXCEPT one, test_datetime.js .
See e.g. https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2a8d826e3cac7bc3c89a8d73e5f3bdb184bc9c75 .

With this patch and shareGlobal=false (current trunk), all tests pass. With this patch but shareGlobal=true the one test fails. However, it fails when using libical backend, when the services aren't unregistered thus the code in this patch is not run. So I assume the problem is not caused by this patch, but by something else affected by the global sharing, which we still need to find.

Comment 145

[Jorg K (CEST = GMT+2)]

Comment on attachment 8933993 [details] [diff] [review]
1401528-MimeTextDecoder.patch

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

I'd prefer to move this to a follow-up bug. Here we should focus on fixing the globals issue.

::: mailnews/mime/jsmime/test/head_xpcshell_glue.js
@@ +44,5 @@
>             .then(contents => callback(undefined, contents), callback);
>    },
>  };
>  requireCache.set("fs", fs);
> +Components.utils.import("resource:///modules/jsmime.jsm");

I'm really the wrong person to review this. I have no idea what the difference is between Services.scriptloader.loadSubScript() and the import. I guess Joshua had his (undocumented) reasons for this.

::: mailnews/mime/src/jsmime.jsm
@@ +54,5 @@
>    },
>  };
>  
>  var {TextDecoder} = Components.utils.getGlobalForObject(
>    Components.utils.import("resource://gre/modules/Services.jsm", {}));

Do we still need this? The JS error came from assigning to the undefined TextDecoder. My rename patch (attachment 8923209 [details] [diff] [review]) didn't have that.

Comment 146

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+1) from comment #145)
> Do we still need this? The JS error came from assigning to the undefined
> TextDecoder. My rename patch (attachment 8923209 [details] [diff] [review])
> didn't have that.
As far as I remember, TextDecoder was known on the LHS of the assignment.

Also, in 
 function FallbackTextDecoder(charset, options) {
   try {
-    return new RealTextDecoder(charset, options);
+    return new TextDecoder(charset, options);
   } catch (e) {
     return new FakeTextDecoder(charset, options);
   }
 }
you need to check manually that the catch block isn't always run as was the case with attachment 8911733 [details] [diff] [review]. That malfunction didn't show up in any test.

Comment 147

[:aceman]

(In reply to Jorg K (GMT+1) from comment #145)
> >  requireCache.set("fs", fs);
> > +Components.utils.import("resource:///modules/jsmime.jsm");
> 
> I'm really the wrong person to review this. I have no idea what the
> difference is between Services.scriptloader.loadSubScript() and the import.
> I guess Joshua had his (undocumented) reasons for this.

The jsmime.jsm loads jsmime.js inside it. It seems to work to import the parent instead of loading the child directly.
We can try waiting for jcranmer, while having the risky code in tree.
 
> >  var {TextDecoder} = Components.utils.getGlobalForObject(
> >    Components.utils.import("resource://gre/modules/Services.jsm", {}));
> 
> Do we still need this? The JS error came from assigning to the undefined
> TextDecoder. My rename patch (attachment 8923209 [details] [diff] [review])
> didn't have that.

I have no idea here, becase there is no TextDecoder in Services.jsm so I do not understand what do we pull from Services.
Some other files seem to get TextDecoder via Cu.importGlobalProperties(["TextDecoder"]);
Many of our files do not import it and just use it directly.

Comment 148

[:aceman]

(In reply to Jorg K (GMT+1) from comment #146)
> Also, in 
>  function FallbackTextDecoder(charset, options) {
>    try {
> -    return new RealTextDecoder(charset, options);
> +    return new TextDecoder(charset, options);
>    } catch (e) {
>      return new FakeTextDecoder(charset, options);
>    }
>  }
> you need to check manually that the catch block isn't always run as was the
> case with attachment 8911733 [details] [diff] [review]. That malfunction
> didn't show up in any test.

Good idea. I removed the TextDecoder declaration/import block and removed the try {} and didn't get any exception in the tests.

Comment 149

[:aceman]

Attachment: 1401528-MimeTextDecoder.patch v2

I could simplify it down to this and the tests still pass.
I don't know whether it is correct and the double 'new' (inside the function and at the caller) seems suspicious to me (but this was there before).

Comment 150

[Jorg K (CEST = GMT+2)]

(In reply to :aceman from comment #147)
> We can try waiting for jcranmer, while having the risky code in tree.
I don't see any risky code. It's no more risky than it has always been.

> I have no idea here, becase there is no TextDecoder in Services.jsm so I do
> not understand what do we pull from Services.
> Some other files seem to get TextDecoder via
> Cu.importGlobalProperties(["TextDecoder"]);
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.importGlobalProperties
says: we can just get ... by importing a known module that has the objects, such as Services.jsm.

> Many of our files do not import it and just use it directly.
As I said, available as LHS, but you can't assign to it.

(In reply to :aceman from comment #148)
> Good idea. I removed the TextDecoder declaration/import block and removed
> the try {} and didn't get any exception in the tests.
The try is there for charsets that the standard TextDecoder doesn't handle, for example UTF-7.
See comment #23, comment #25 and bug 1402813. If the code is right, unlike attachment 8911733 [details] [diff] [review], the catch block is usually not triggered.

Comment 151

[Jorg K (CEST = GMT+2)]

Coming to look at the last failure. With the patch from bug 1422547 applied,
  mozilla/mach xpcshell-test calendar/test/unit/test_datetime.js
fails in the libical part with:
[calBackendLoader] Using Thunderbird's builtin libical backend
...
really_run_test FAIL [really_run_test : 77] "America/New_York" == "America/New York"

Strangely, that test failure wasn't present originally, see comment #0, and it's also not present in the try run quoted in comment #13.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=86f33c2e7d20b028d14051423db2b70612011fbe

Weird.

Comment 152

[:aceman]

(In reply to Jorg K (GMT+1) from comment #142)
> Since we're nearing the end here and Aceman did most the work, it's fair to
> give him the bug.

With much help from many others involved here, like you.

Comment 153

[Tooru Fujisawa [:arai]]

Comment on attachment 8933993 [details] [diff] [review]
1401528-MimeTextDecoder.patch

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

::: mailnews/mime/src/jsmime.jsm
@@ +54,5 @@
>    },
>  };
>  
>  var {TextDecoder} = Components.utils.getGlobalForObject(
>    Components.utils.import("resource://gre/modules/Services.jsm", {}));

if the actual TextDecoder in this context works as expected and this was neccessary just because this file was overwriting with another value, this could be removed.

Comment 154

[Tooru Fujisawa [:arai]]

Comment on attachment 8934011 [details] [diff] [review]
1401528-MimeTextDecoder.patch v2

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

yes, this should be fine

Comment 155

[Jorg K (CEST = GMT+2)]

Comment on attachment 8934011 [details] [diff] [review]
1401528-MimeTextDecoder.patch v2

This patch simplifies things a lot. I hope we can get a word from Joshua who is extremely busy with non-TB stuff these days. I think it would be best to move this patch to a new bug "Simplify handling of TextDecoder in JS Mime after bug 1401528". I hope to finish off this bug soon and we don't want side issues to cause a delay here. But anyway, we can wait until we have a fix for test_datetime.js, see comment #151.

Comment 156

[Jorg K (CEST = GMT+2)]

Coming back to
  mozilla/mach xpcshell-test calendar/test/unit/test_datetime.js

After applying attachment 8933994 [details] [diff] [review] this test doesn't fail when jsloader.shareGlobal is false, but it fails when it's true:
[calBackendLoader] Using Thunderbird's builtin libical backend
...
really_run_test FAIL [really_run_test : 77] "America/New_York" == "America/New York"

Comment 157

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8933994 [details] [diff] [review]
1401528-calendar.patch [landed comment #159]

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

Thanks for finding the fix here, r=philipp

Comment 158

[Jorg K (CEST = GMT+2)]

Philipp, any idea why test_datetime.js fails, see comment #156?

Comment 159

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/b38e2320edda
fix unregistering factories in calBackendLoader.js. r=philipp

Comment 160

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to Jorg K (GMT+1) from comment #158)
> Philipp, any idea why test_datetime.js fails, see comment #156?

Looks like a the localized string is being used instead of the id. I'm not sure why this would change with a shared global, but I think we can handle that in a new bug.

Comment 161

[Jorg K (CEST = GMT+2)]

As per Philipp's suggestion, let's move the rest to a new bug. I'm going to back out the temporary fix and we're done here.

Comment 162

[:aceman]

If this is the last failing test, I also think we should enable the sharedGlobal pref if we want it for TB 59. It would be high time now. There may be other problems not detected by the tests but could be found by running with this change locally (nightly testers).

Comment 163

[Jorg K (CEST = GMT+2)]

Comment on attachment 8933993 [details] [diff] [review]
1401528-MimeTextDecoder.patch

Let's move this to a new bug: "Simplify handling of TextDecoder in JS Mime after bug 1401528"

Comment 164

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4d8fb3c5fcd0
Backed out changeset 4de5f6029108, setting of jsloader.shareGlobal, since it was a temporary fix. a=jorgk

Comment 165

[Jorg K (CEST = GMT+2)]

We'll meet again in bug 1424350 to fix test_datetime.js.

Comment 166

[:aceman]

(In reply to alta88 from comment #90)
> > The decoded code uses the Array global, and in that context |this| is the
> > global object. I'm not getting this locally, can you file a new bug with
> > more details if this is still an issue?
> 
> There is a similar obfuscated app secret in nsBox.js which fails in the same
> way.
> https://dxr.mozilla.org/comm-central/rev/
> 306fff0f8a9d1fe8b7eeafd86c68166d78288767/mail/components/cloudfile/nsBox.
> js#810

Thanks, bug 1427297.