Open Bug 1536779 Opened 7 months ago Updated 7 months ago

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

Categories

(SeaMonkey :: MailNews: Composition, enhancement)

enhancement
Not set

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

Details

Attachments

(1 file)

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
Summary: Port Bug 1528840 - addressingWidgetOverlay.js, abMailListDialog.js code cleanup → Port |Bug 1528840 - addressingWidgetOverlay.js, abMailListDialog.js code cleanup| to SeaMonkey
Attachment #9052225 - Flags: review?(frgrahl)
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 on attachment 9052225 [details] [diff] [review]
Port changes to SM

Clearing review for now per IRC. Waiting for next version.
Attachment #9052225 - Flags: review?(frgrahl)

(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 on attachment 9052225 [details] [diff] [review]
Port changes to SM

As there is only one caller awTabFromRecipient could be inlined.
Attachment #9052225 - Flags: review?(frgrahl)
You need to log in before you can comment on or make changes to this bug.