Last Comment Bug 808172 - convert Mutation Events to use MutationObserver in mail/
: convert Mutation Events to use MutationObserver in mail/
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 20.0
Assigned To: Magnus Melin
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-02 13:39 PDT by Magnus Melin
Modified: 2012-11-22 12:16 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (10.90 KB, patch)
2012-11-02 13:39 PDT, Magnus Melin
mconley: review+
Details | Diff | Review
proposed fix, v2 (12.99 KB, patch)
2012-11-19 11:17 PST, Magnus Melin
mconley: review+
Details | Diff | Review

Description Magnus Melin 2012-11-02 13:39:03 PDT
Created attachment 677870 [details] [diff] [review]
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) - (Away until June 29th) 2012-11-16 14:01:46 PST
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 2012-11-17 13:00:02 PST
https://hg.mozilla.org/comm-central/rev/f61698f08e77 -> FIXED
Comment 3 Mike Conley (:mconley) - (Away until June 29th) 2012-11-17 14:02:43 PST
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 2012-11-19 11:17:18 PST
Created attachment 683226 [details] [diff] [review]
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) - (Away until June 29th) 2012-11-21 20:02:30 PST
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 2012-11-22 12:04:38 PST
https://hg.mozilla.org/comm-central/rev/a9d426e01d37 -> FIXED

Note You need to log in before you can comment on or make changes to this bug.