Closed Bug 1294526 Opened 5 years ago Closed 5 years ago

Check for unexpected hide events in removeGrandChildrenNHideParent

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: michael.li11702, Assigned: michael.li11702)

Details

Attachments

(1 file)

Add checks to make sure grandchildren don't fire hide events since the parent will be hidden anyway.
Assignee: nobody → mili
Comment on attachment 8780273 [details]
Bug 1294526: Check for unexpected hide/reorder events in removeGrandChildrenNHideParent

https://reviewboard.mozilla.org/r/70970/#review68488

::: accessible/tests/mochitest/events/test_coalescence.html:326
(Diff revision 1)
>        this.eventSeq = [
>          new invokerChecker(EVENT_HIDE, getAccessible(aParentId)),
> -        new invokerChecker(EVENT_REORDER, getNode(aParentId).parentNode)
> +        new invokerChecker(EVENT_REORDER, getNode(aParentId).parentNode),
> +        new unexpectedInvokerChecker(EVENT_HIDE, getAccessible(aChild1Id)),
> +        new unexpectedInvokerChecker(EVENT_HIDE, getAccessible(aChild2Id))
>        ];

it might be nice to check there's no reorder on parent too, otherwise looks ok
Attachment #8780273 - Flags: review?(surkov.alexander) → review+
Comment on attachment 8780273 [details]
Bug 1294526: Check for unexpected hide/reorder events in removeGrandChildrenNHideParent

https://reviewboard.mozilla.org/r/70970/#review68488

> it might be nice to check there's no reorder on parent too, otherwise looks ok

Do you mean check for no reorders on the two childs? Since we expect the reorder on the parent.
(In reply to Michael Li from comment #3)
> Comment on attachment 8780273 [details]
> Bug 1294526: Check for unexpected hide/reorder events in
> removeGrandChildrenNHideParent
> 
> https://reviewboard.mozilla.org/r/70970/#review68488
> 
> > it might be nice to check there's no reorder on parent too, otherwise looks ok
> 
> Do you mean check for no reorders on the two childs? Since we expect the
> reorder on the parent.

nope, there is a reorder event for parent's parent, and reorder on parent is unexpected.
Does it look good now? I'm not sure if parentNode vs. parentElement would make a difference.
Flags: needinfo?(surkov.alexander)
(In reply to Michael Li from comment #7)
> Does it look good now? I'm not sure if parentNode vs. parentElement would
> make a difference.

aChild1Id and aChild2Id have common parent, which is aParentId, so only one checker for aParentId is needed
Flags: needinfo?(surkov.alexander)
(In reply to alexander :surkov from comment #8)
> (In reply to Michael Li from comment #7)
> > Does it look good now? I'm not sure if parentNode vs. parentElement would
> > make a difference.
> 
> aChild1Id and aChild2Id have common parent, which is aParentId, so only one
> checker for aParentId is needed

Oh I see, so we're checking to make sure there's no extra reorder on aParentId.
Never mind, I didn't realize the expected reorder is for aParentId's parent, oops.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/608c36e6b0f7
Check for unexpected hide/reorder events in removeGrandChildrenNHideParent. r=surkov
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/608c36e6b0f7
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.