Closed Bug 1538549 Opened 6 years ago Closed 6 years ago

[de-xbl] convert contact binding to <richlistitem is="chat-contact">

Categories

(Thunderbird :: Instant Messaging, task)

Product:
Thunderbird
Component:
Instant Messaging
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 69.0

People

(Reporter: mkmelin, Assigned: khushil324)

References

Details

Attachments

(1 file, 2 obsolete files)

Bug-1538549_de-xbl_imcontact.patch
(deleted), patch
khushil324
: review+
Details | Diff | Splinter Review
Attached patch de-xbl_imcontact.patch (obsolete) (deleted) — — Splinter Review
Attachment #9056230 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9056230 [details] [diff] [review] de-xbl_imcontact.patch Review of attachment 9056230 [details] [diff] [review]: ----------------------------------------------------------------- Should hg mv --after mail/components/im/content/imcontact.xml mail/components/im/content/chat-contact.js This looks good overall I think ::: mail/components/im/content/chat-contact.js @@ +12,5 @@ > + /** > + * The MozChatContact widget displays contact information about user under > + * chat-groups, online contacts and offline contacts: i.e. icon and username. > + * On double clicking the element, it gets removed from chat-groups and chat-conv > + * element gets added under chat-group named conversation. ... it gets moved into the conversations @@ +45,5 @@ > + }); > + > + this.addEventListener("mousedown", (event) => { > + if (!this.hasAttribute("aliasing") && this.canOpenConversation() && > + event.originalTarget.classList.contains("startChatBubble")) { for these things we indent if (!this.hasAttribute("aliasing") && this.canOpenConversation() && event.originalTarget.classList.contains("startChatBubble")) { @@ +54,5 @@ > + > + this.addEventListener("click", (event) => { > + if (!this.hasAttribute("aliasing") && this.canOpenConversation() && > + event.detail == 2 && > + event.originalTarget.getAttribute("anonid") != "expander") { is this dead code? I don't see an anonid="expander" in the tree anywhere ::: mail/components/im/content/chat-messenger.js @@ +19,5 @@ > this.menu = aXulMenu; > let localName = this.target.localName; > + document.getElementById("contactlistbox").selectedItem = this.target; > + this.onContact = localName == "richlistitem" && > + this.target.getAttribute("is") == "chat-contact"; would add some parenthesis here for readability
Attachment #9056230 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 9056230 [details] [diff] [review] de-xbl_imcontact.patch Review of attachment 9056230 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/components/im/content/chat-contact.js @@ +159,5 @@ > + }).bind(this); > + this.parentNode.addEventListener("select", this._parentSelectListener); > + } > + > + finishAliasing(aSave) { and please make this save instead of aSave
Attachment #9057061 - Attachment is obsolete: true
Attachment #9057110 - Flags: review+

Patch updated with ESLint correction. Magnus, can you confirm the patch after seeing the test results on try-comm?

NOt the best of try runs but looks ok to me (and the linting failure is now fixed)

Type: defect → task
Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d760edcda279
[de-xbl] convert contact binding to custom element. r=mkmelin DONTBUILD

Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 68.0
Regressions: 1543906

This never made it into esr 68. I'm not sure why.

Target Milestone: Thunderbird 68.0 → Thunderbird 67.0
Target Milestone: Thunderbird 67.0 → Thunderbird 69.0

There were other related xbl elements: chat-group and chat-conversation. We need to merge all three or none of them as they are co-related. And some other functionalities were dependent upon that. So we didn't merge these patches.

I see now. I was looking at the wrong time window. It all makes sense.

Also keep in mind that chat in TB 68 is working based on a series of backouts, see bug 1555136 comment #21. Please don't touch chat in TB 68 or it will all fall apart.

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

Attachment

General

Creator:
Created:
Updated:
Size: