354745 - Show/hide events not fired for layout changes in a changelist

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

When frames change visibility or display indirectly, e.g. via an ancestor class/attribute change which causes computed style to change, the accessibility show/hide/reorder events are not getting fired.

Comment 1

[Aaron Leventhal]

Attachment: Testcase

Comment 2

[Aaron Leventhal]

Attachment: Change where we check for significant changes to frames

Comment 3

[Boris Zbarsky [:bzbarsky]]

I won't be able to get to this for about a week.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Doesn't that bring us back to the situation where the change type can lie?  That is, we could have an EVENT_HIDE on a node when what we're actually doing is showing stuff we didn't show before:

<div style="visibility: visible">
  <div style="visibility: hidden">
    Content
  </div>
</div>

and changing the visibility values will fire HIDE while showing the content.  Is that actually correct?  If so, there should be some major comments in the code explaining why.  I know this has come up before, and I don't recall seeing a sateisfactory answer...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 240527 [details] [diff] [review]
Change where we check for significant changes to frames

With those detailed comments added, looks ok.  Watch the perf numbers very carefully.

Comment 6

[Aaron Leventhal]

Boris, it looks like what we're doing for display:none is correct, but for visibility:visible is not correct. Now that I look at it, I see that visibility is reversed when a visibility:hidden element has a visibility:visible child. This is not like display:none -- if it has a display:block child the child is still not visible. The entire subtree is now gone.

I'd like to file a follow up bug for dealing with those issues. This fixes a topcrash do I'd like to get it in now.

Comment 7

[Aaron Leventhal]

Checked in.

Spun off bug 355521 for the visibility issues. Boris, you're correct, we need to handle visibility differently. Thank you for explaining that.

Comment 8

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381