1536779 - Port |Bug 1528840 - addressingWidgetOverlay.js, abMailListDialog.js code cleanup| to SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Composition, enhancement)

Description

[Ian Neal]

As mentioned over in Bug 1528840:

• awInputChanged function is unused
• top.doNotCreateANewRow is therefore unused too
• event.keyCode is depreciated so replace with event.key

Comment 1

[Ian Neal]

Attachment: Port changes to SM

Comment 2

[Thomas D. (:thomas8)]

Comment on attachment 9052225 [details] [diff] [review]
Port changes to SM

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

Thanks for starting out to port this. However, it seems you only ported a small subset of the TB changes, yet there's more which equally applies to SM. I guess whilst were at it, let's try to go all the way and keep things in sync. Removing clutter in this corner is so helpful to understand the code better and improve it.

::: suite/mailnews/components/compose/content/addressingWidgetOverlay.js
@@ -657,5 @@
>    var row = awGetRowByInputElement(element);
>    if (!event.shiftKey && row < top.MAX_RECIPIENTS) {
>      var listBoxRow = row - 1; // listbox row indices are 0-based, ours are 1-based.
>      var listBox = document.getElementById("addressingWidget");
>      listBox.listBoxObject.ensureIndexIsVisible(listBoxRow + 1);

For cases of focus change, apparently ensureIndexIsVisible happens inbuilt from respective elements. For TB I found that most of these are dispensible. Not for SM?

Then after removing this block, there's one line left in this function (the spellcheck), so easy to eliminate and do directly in the only caller, awRecipientKeyPress

https://searchfox.org/comm-central/source/suite/mailnews/components/compose/content/addressingWidgetOverlay.js#771

There's a lot more useless helper functions (as seen on TB) which ultimately do nothing but ensureIndexIsVisible, which is no longer needed. 
So unless I am missing something, why don't you port much more of my patch attachment 9052052 [details] [diff] [review] to SM?

Comment 3

[Frank-Rainer Grahl (:frg)]

Comment on attachment 9052225 [details] [diff] [review]
Port changes to SM

Clearing review for now per IRC. Waiting for next version.

Comment 4

[Ian Neal]

(In reply to Thomas D. from comment #2)

For cases of focus change, apparently ensureIndexIsVisible happens inbuilt
from respective elements. For TB I found that most of these are dispensible.
Not for SM?

As SM still uses listbox, if ensureIndexIsVisible is removed then Tab doesn't move to an element which is not visible in the addressing widget.

Comment 5

[Ian Neal]

Comment on attachment 9052225 [details] [diff] [review]
Port changes to SM

As there is only one caller awTabFromRecipient could be inlined.

Comment 6

[Frank-Rainer Grahl (:frg)]

Comment on attachment 9052225 [details] [diff] [review]
Port changes to SM

Good start. Can go into 2.53 and 2.57 too. Happy to see more removals as Thomas suggested.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/25c5e757538e
Port |Bug 1528840 - addressingWidgetOverlay.js, abMailListDialog.js code cleanup| to SeaMonkey r=frg

Comment 8

[Frank-Rainer Grahl (:frg)]

https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/7d36c07128a589a5bde03b93d998714770edebbc
Port |Bug 1528840 - addressingWidgetOverlay.js, abMailListDialog.js code cleanup| to SeaMonkey r=frg a=frg
Target 2.53.4