Once bug 1472555 lands, it will be possible to flatten the "richlistbox" bindings and remove "listbox.xml" entirely.

Comment on attachment 8994173 [details] Bug 1476611 - Part 2 - Flatten the "richlistbox" bindings. https://reviewboard.mozilla.org/r/258778/#review265664 Code analysis found 7 defects in this patch: - 7 defects found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: toolkit/content/widgets/richlistbox.xml:263 (Diff revision 1) > + ]]> > + </setter> > + </property> > + > + <!-- nsIDOMXULMultiSelectControlElement --> > + <field name="selectedItems">new ChromeNodeList()</field> Error: 'ChromeNodeList' is not defined. [eslint: no-undef] ::: toolkit/content/widgets/richlistbox.xml:853 (Diff revision 1) > + <handler event="keypress" phase="target"> > + <![CDATA[ > + if (this.disableKeyNavigation || !event.charCode || > + event.altKey || event.ctrlKey || event.metaKey) > + return; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:856 (Diff revision 1) > + event.altKey || event.ctrlKey || event.metaKey) > + return; > + > + if (event.timeStamp - this._lastKeyTime > 1000) > + this._incrementalString = ""; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:860 (Diff revision 1) > + this._incrementalString = ""; > + > + var key = String.fromCharCode(event.charCode).toLowerCase(); > + this._incrementalString += key; > + this._lastKeyTime = event.timeStamp; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:866 (Diff revision 1) > + // If all letters in the incremental string are the same, just > + // try to match the first one > + var incrementalString = /^(.)\1+$/.test(this._incrementalString) ? > + RegExp.$1 : this._incrementalString; > + var length = incrementalString.length; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:874 (Diff revision 1) > + var start = l > 0 ? this.getIndexOfItem(this.selectedItems[l - 1]) : -1; > + // start from the first element if none was selected or from the one > + // following the selected one if it's a new or a repeated-letter search > + if (start == -1 || length == 1) > + start++; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:1073 (Diff revision 1) > + } else { > + /* We want to deselect all the selected items except what was > + clicked, UNLESS it was a right-click. We have to do this > + in click rather than mousedown so that you can drag a > + selected group of items */ > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

Comment on attachment 8994173 [details] Bug 1476611 - Part 2 - Flatten the "richlistbox" bindings. https://reviewboard.mozilla.org/r/258778/#review265666 Code analysis found 7 defects in this patch: - 7 defects found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: toolkit/content/widgets/richlistbox.xml:263 (Diff revision 2) > + ]]> > + </setter> > + </property> > + > + <!-- nsIDOMXULMultiSelectControlElement --> > + <field name="selectedItems">new ChromeNodeList()</field> Error: 'ChromeNodeList' is not defined. [eslint: no-undef] ::: toolkit/content/widgets/richlistbox.xml:853 (Diff revision 2) > + <handler event="keypress" phase="target"> > + <![CDATA[ > + if (this.disableKeyNavigation || !event.charCode || > + event.altKey || event.ctrlKey || event.metaKey) > + return; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:856 (Diff revision 2) > + event.altKey || event.ctrlKey || event.metaKey) > + return; > + > + if (event.timeStamp - this._lastKeyTime > 1000) > + this._incrementalString = ""; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:860 (Diff revision 2) > + this._incrementalString = ""; > + > + var key = String.fromCharCode(event.charCode).toLowerCase(); > + this._incrementalString += key; > + this._lastKeyTime = event.timeStamp; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:866 (Diff revision 2) > + // If all letters in the incremental string are the same, just > + // try to match the first one > + var incrementalString = /^(.)\1+$/.test(this._incrementalString) ? > + RegExp.$1 : this._incrementalString; > + var length = incrementalString.length; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:874 (Diff revision 2) > + var start = l > 0 ? this.getIndexOfItem(this.selectedItems[l - 1]) : -1; > + // start from the first element if none was selected or from the one > + // following the selected one if it's a new or a repeated-letter search > + if (start == -1 || length == 1) > + start++; > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces] ::: toolkit/content/widgets/richlistbox.xml:1073 (Diff revision 2) > + } else { > + /* We want to deselect all the selected items except what was > + clicked, UNLESS it was a right-click. We have to do this > + in click rather than mousedown so that you can drag a > + selected group of items */ > + Error: Trailing spaces not allowed. [eslint: no-trailing-spaces]

Comment on attachment 8994173 [details] Bug 1476611 - Part 2 - Flatten the "richlistbox" bindings. https://reviewboard.mozilla.org/r/258778/#review265690 I wish we could see cross-file line moves in review tools - I've spot checked but am assuming this is a direct copy/paste fold. Let me know if there's anything more going on with this that you want me to review.

Comment on attachment 8994174 [details] Bug 1476611 - Part 1 - Remove unused listbox methods. https://reviewboard.mozilla.org/r/258780/#review265692

CCing some Thunderbird folks - AFAICT the richlistbox methods being removed in part 1 are either unused in c-c or are replicated in separate bindings files. Part 2 removes listbox-base and listitem-base since there's now only one binding extending them in m-c, so if you have other bindings that extend them you may want to restore the bindings in c-c.

Filed bug 1477777 for the C-C work. Thanks for the heads-up.

(In reply to Brian Grinstead [:bgrins] from comment #7) > I wish we could see cross-file line moves in review tools - I've spot > checked but am assuming this is a direct copy/paste fold. Yes, the second part just moves methods across files. In the first part I accidentally removed two methods that were still used, I've re-added them and started a tryserver build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d7cdd7890a5218588a68c74d368190372ab8bfd

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/162e063abd54 Part 1 - Remove unused listbox methods. r=bgrins https://hg.mozilla.org/integration/mozilla-inbound/rev/a0f8e3ef3a72 Part 2 - Flatten the "richlistbox" bindings. r=bgrins

https://hg.mozilla.org/mozilla-central/rev/162e063abd54 https://hg.mozilla.org/mozilla-central/rev/a0f8e3ef3a72