Closed Bug 955564 Opened 10 years ago Closed 10 years ago

Simplify system message collapsing

Categories

(Instantbird Graveyard :: Conversation, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: florian)

References

Details

Attachments

(2 files, 1 obsolete file)

*** Original post on bio 2126 at 2013-08-25 17:34:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Blocks: 955182
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 2126 as attmnt 2783 at 2013-08-25 17:34:00 UTC ***

This is a port (further simplified!) of Mook's patch from https://bugzilla.mozilla.org/show_bug.cgi?id=896216

This became necessary for Bubbles after bug 955182 (bio 1750) as the existing JS code of Bubbles assumes that when receiving a mutation event, the new node is the last one... which is no longer true as mutation observers batch the mutation notifications.
Attachment #8354552 - Flags: review?(aleth)
Comment on attachment 8354552 [details] [diff] [review]
Patch

*** Original change on bio 2126 attmnt 2783 at 2013-08-26 13:16:47 UTC ***

This looks like a great simplification :)

>   else if (target.tagName == "P" && target.className == "event") {
>-    groupEvents(target.parentNode);
>+    let parent = target.parentNode;
>+    // We need to start a group with this element if there are at least 4
>+    // system messages and they aren't already grouped.
>+    if (!parent.querySelector(".eventToggle") &&

Would it be more efficient to put an "eventToggle" attribute on the parent to avoid having to do querySelector?

>+        parent.querySelector("p.event:nth-of-type(4)")) {
>+      var p = document.createElement("p");
>+      p.setAttribute("class", "eventToggle");

p.className = "eventToggle";
Attachment #8354552 - Flags: review?(aleth) → review-
Attached patch Patch v2Splinter Review
*** Original post on bio 2126 as attmnt 2785 at 2013-08-26 18:04:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354554 - Flags: review?(aleth)
Attached patch InterdiffSplinter Review
*** Original post on bio 2126 as attmnt 2786 at 2013-08-26 18:04:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Comment on attachment 8354552 [details] [diff] [review]
Patch

*** Original change on bio 2126 attmnt 2783 at 2013-08-26 18:04:04 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354552 - Attachment is obsolete: true
Assignee: nobody → florian
Comment on attachment 8354554 [details] [diff] [review]
Patch v2

*** Original change on bio 2126 attmnt 2785 at 2013-08-28 08:42:33 UTC ***

Thanks!
Attachment #8354554 - Flags: review?(aleth) → review+
OS: Other → All
Hardware: x86 → All
Whiteboard: [checkin-needed]
*** Original post on bio 2126 at 2013-08-28 08:53:37 UTC ***

http://hg.instantbird.org/instantbird/rev/fba0f1694de8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
You need to log in before you can comment on or make changes to this bug.