808172 - convert Mutation Events to use MutationObserver in mail/

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Check the error console. You'll see a warning

Warning: Use of Mutation Events is deprecated. Use MutationObserver instead.
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 944

This patch get rid of the mutation events, should be a small perf win. I didn't touch the im/ parts, at least yet.

The _setMenuitemAttributes method is (almost) copy-pasted from firefox.

(Firefox doesn't yes use a mutationobserver in tabbrowser.xml looks like they manually call a method to produce TabAttrModified events which looks tedious.)

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 677870 [details] [diff] [review]
proposed fix

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

Looks good to me - glad to see old cruft getting removed. Just a style nit. r=me with that fixed. Thanks!

::: mail/base/content/msgMail3PaneWindow.js
@@ +931,5 @@
> +  // whenever attributes on the columns change.
> +  let observer = new MutationObserver(function handleMutations(mutations) {
> +    gFolderDisplay.hintColumnsChanged();
> +  });
> +  observer.observe(document.getElementById("threadCols"),

I'd prefer the style I laid out in tabmail.xml.

::: mail/base/content/tabmail.xml
@@ +2526,5 @@
>          var tabcontainer = tabmail.tabContainer;
>          var tabs = tabcontainer.childNodes;
>  
>          // Listen for changes in the tab bar.
> +        this._mutationObserver.observe(tabcontainer,

I think I'd prefer:

this._mutationObserver.observe(tabcontainer, {
  attributes: true,
  subtree: true,
  attributeFilter: ["label", ...];
});

Comment 2

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/f61698f08e77 -> FIXED

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Backed out due to suspected Mozmill perma-orange:

https://hg.mozilla.org/comm-central/rev/ee9a1e889d12

TEST-START | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test_column_defaults_inherit_from_inbox
Test Failure: Found visible column 'dateCol' but was expecting 'tagsCol'!
desired list: threadCol,flaggedCol,attachmentCol,subjectCol,unreadButtonColHeader,senderCol,junkStatusCol,tagsCol
 actual list: threadCol,flaggedCol,attachmentCol,subjectCol,unreadButtonColHeader,senderCol,junkStatusCol,dateCol
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test-columns.js::test_column_defaults_inherit_from_inbox
TEST-START | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test_column_visibility_persists_through_tab_changes
Test Failure: Found visible column 'dateCol' but was expecting 'tagsCol'!
desired list: threadCol,flaggedCol,attachmentCol,subjectCol,unreadButtonColHeader,senderCol,junkStatusCol,tagsCol
 actual list: threadCol,flaggedCol,attachmentCol,subjectCol,unreadButtonColHeader,senderCol,junkStatusCol,dateCol
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test-columns.js::test_column_visibility_persists_through_tab_changes
TEST-START | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test_column_visibility_persists_through_folder_changes
Test Failure: Found visible column 'dateCol' but was expecting 'tagsCol'!
desired list: threadCol,flaggedCol,attachmentCol,subjectCol,unreadButtonColHeader,senderCol,junkStatusCol,tagsCol
 actual list: threadCol,flaggedCol,attachmentCol,subjectCol,unreadButtonColHeader,senderCol,junkStatusCol,dateCol
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test-columns.js::test_column_visibility_persists_through_folder_changes
TEST-START | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test_column_reordering_persists
Test Failure: Found visible column 'dateCol' but was expecting 'tagsCol'!
desired list: threadCol,flaggedCol,attachmentCol,senderCol,subjectCol,unreadButtonColHeader,junkStatusCol,tagsCol
 actual list: threadCol,flaggedCol,attachmentCol,senderCol,subjectCol,unreadButtonColHeader,junkStatusCol,dateCol
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test-columns.js::test_column_reordering_persists
TEST-START | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test_reset_to_inbox
Test Failure: Found visible column 'senderCol' but was expecting 'subjectCol'!
desired list: threadCol,flaggedCol,attachmentCol,subjectCol,unreadButtonColHeader,senderCol,junkStatusCol,dateCol
 actual list: threadCol,flaggedCol,attachmentCol,senderCol,subjectCol,unreadButtonColHeader,junkStatusCol,dateCol
TEST-UNEXPECTED-FAIL | /Users/cltbld/talos-slave/test/build/mozmill/folder-display/test-columns.js | test-columns.js::test_reset_to_inbox

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

folderDisplay.js was hacking around getting dom modifications as separate events, with mutations you get them all in one call. The other changes are as before.

Try run coming in here: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=825399b83f9f

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 683226 [details] [diff] [review]
proposed fix, v2

Good stuff. folder-display tests pass for me locally. Let's try this again.

Comment 6

[Magnus Melin [:mkmelin]]

https://hg.mozilla.org/comm-central/rev/a9d426e01d37 -> FIXED

Comment 7

[Philippe Lefèvre]

Issue still present into TDB 45.8.0

Horodatage : 23/06/2017 08:09:18
Avertissement : L'utilisation des « Mutation Events » est obsolète. Utiliser « MutationObserver » à la place.
Fichier source : chrome://calendar/content/widgets/calendar-widgets.xml
Ligne : 495

Comment 8

[Magnus Melin [:mkmelin]]

That's in calendar. Filed bug 1376512 about it.