513213 - coalesce events when new event is appended to the queue

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

This should help to improve the situation described in bug 451362. However this might help to coalesce reorder events only. It doesn't address remove or hide events. For example:

1. child remove, parent remove -> two remove events, one reorder for parent's parent
2. child hide, parent remove -> hide and remove events, one reorder for parent's parent
3. child remove, parent hide -> remove and hide events, one reorder for parent's parent

Here this bug fixed reorder events issue for the case #1. The meantime we get two reorder events, one is for parent, another is parent's parent.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

So for reorder events is the idea to only fire the reorder on the oldest ancestor?

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> So for reorder events is the idea to only fire the reorder on the oldest
> ancestor?

For the topest, the same like for any other coalescent events if I follow your question right.

Comment 4

[Marco Zehe (:MarcoZ)]

Comment on attachment 397254 [details] [diff] [review]
patch

r=me for the test part with a few nits:
>+    function coalescenseBase(aChildAction, aParentAction,

According to David, this is spelled "coalescence" (ce at the end). Please change.

>+        // unexpected evetns

t and n need to be swapped :)

>+    function removeOrHideCoalescenseBase(aChidID, aParentID,

You're using "aChidId" in a lot of places, but not everywhere. I suggest to correct to "aChildId" and be consistent. ;)

>+     * Hide parnet node and then remove its child node.

"parent".

>+     * Remove parnet node and then hide its child node.

The same.

Can you kick off a try server build so I can play with it some?

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 397254 [details] [diff] [review]
patch

r=me.

I had many questions but eventually answered them. I want a bigger brain.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

The only possible perf hit I could see was that we have to set up a function call each time an event is added, whereas we removed the dupes etc in a for loop before.

Comment 7

[alexander :surkov (:asurkov)]

(In reply to comment #4)

> Can you kick off a try server build so I can play with it some?

Sure - https://build.mozilla.org/tryserver-builds/[e-mail]-try-ea1429164cc2/

Comment 8

[Marco Zehe (:MarcoZ)]

Comment on attachment 397254 [details] [diff] [review]
patch

This works as expected, and manual tests didn't show any regressions. r=me, thanks!

Comment 9

[alexander :surkov (:asurkov)]

landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/dfa26670be9f

Comment 10

[Boris Zbarsky [:bzbarsky]]

This causes noticeable performance problems; see bug 522847.