Closed Bug 354745 Opened 13 years ago Closed 13 years ago

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


(Core :: Disability Access APIs, defect, major)

Not set





(Reporter: aaronlev, Assigned: aaronlev)



(Keywords: access, crash)


(2 files)

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.
Attached file Testcase
Attachment #240527 - Flags: superreview?(bzbarsky)
Attachment #240527 - Flags: review?(bzbarsky)
I won't be able to get to this for about a week.
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">

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 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.
Attachment #240527 - Flags: superreview?(bzbarsky)
Attachment #240527 - Flags: superreview+
Attachment #240527 - Flags: review?(bzbarsky)
Attachment #240527 - Flags: review+
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.
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.
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.