The default bug view has changed. See this FAQ.

Image frame changes via :before/:after causes recreation of accessible objects

NEW
Unassigned

Status

()

Core
Disability Access APIs
--
major
10 years ago
5 years ago

People

(Reporter: Scott Haeger, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {access})

Trunk
x86
Linux
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
This checkbox test case https://bugzilla.mozilla.org/attachment.cgi?id=280484 (you will need the three images) recreates the ATK object upon a state change.  Do the following to test.

1) Open up the test case and note that the checkboxes are visible.
2) Open up Accerciser and find one of the checkboxes.
3) Click on the checkbox to cause a state change.  You will notice that the checkbox goes defunct and that a new object is created.
(Reporter)

Updated

10 years ago
Blocks: 374212

Updated

10 years ago
Blocks: 396346

Updated

10 years ago
Severity: normal → major
Whiteboard: orca:high

Comment 1

10 years ago
Actually the root cause is that we are firing an extra internal invalidation event even though the frame didn't actually change -- it was just in a method where it could change.

Comment 2

10 years ago
Created attachment 286033 [details] [diff] [review]
If frame doesn't actually change, don't fire notification to invalidate a11y tree
Attachment #286033 - Flags: superreview?(bzbarsky)
Attachment #286033 - Flags: review?(bzbarsky)
I don't see how that test could possibly be true here.  What am I missing?

Comment 4

10 years ago
There's a <span> with a :before style rule. The :before child frame changes from checked.gif to unchecked.gif, but the frame for the span doesn't actually change.
> There's a <span> with a :before style rule. The :before child frame changes

It usually can't change without a frame reconstruct on the span's frame.  What are the two frames and aContent in RecreateFramesForContent in this case?

Comment 6

10 years ago
aContent is for the <span>
Both frame and newFrame are the same inline frame for that span.

That should not be happening.  If it is, something is seriously buggy.  Note that as far as the frame constructor is concerned, this function is exactly equivalent to aContent being removed from the document and then reinserted into the document...
Oh, actually...  Are you _sure_ that they're "the same inline frame"?  Or is it that |frame| gets destroyed by the ContentRemoved() call, the memory gets placed on the recycling allocator freelist, and we allocate the new frame for the span off that freelist, which puts it at the same memory location as the old one used to occupy?

This chain of events I can absolutely see happening, in which case your equality test will sometimes randomly test true: you're comparing a pointer to deallocated memory with a pointer to newly-allocated memory, and sometimes you'll happen to get equality.

But make no mistake.  This is not a method in which the frame "could change".  This is a method in which the frame is deallocated, with the memory scribbled on with an 0xdddddddd pattern in a debug build, and another frame is possibly allocated.  If there is some circumstance where that does not happen, that's a serious bug.

Of course, using the |frame| pointer for anything other than maybe assigning some other frame pointer to it after the ContentRemoved() call is also a serious bug.
Comment on attachment 286033 [details] [diff] [review]
If frame doesn't actually change, don't fire notification to invalidate a11y tree

This patch is wrong.  If it doesn't break our existing a11y tests, that means we don't have the right tests.
Attachment #286033 - Flags: superreview?(bzbarsky)
Attachment #286033 - Flags: superreview-
Attachment #286033 - Flags: review?(bzbarsky)
Attachment #286033 - Flags: review-

Comment 10

10 years ago
Created attachment 286159 [details] [diff] [review]
If frame *type* doesn't actually change, don't fire notification to invalidate a11y tree

Thanks Bz. The type of accessible object can sometimes be dependent on the frame type (not just the element's markup), so if the markup doesn't change and the frame type doesn't change, there is no need to fire the a11y invalidation.
Attachment #286033 - Attachment is obsolete: true
Attachment #286159 - Flags: superreview?(bzbarsky)
Attachment #286159 - Flags: review?(bzbarsky)
Comment on attachment 286159 [details] [diff] [review]
If frame *type* doesn't actually change, don't fire notification to invalidate a11y tree

So you don't care if some child frame of oldFrame/newFrame changed type?

Comment 12

10 years ago
Boris, how do I test that?
Simplest way is to enable a sheet that, in a single stylesheet:

1)  Changes the :before content for the span
2)  Changes the style for some child of the span (e.g sets it to display:none).

Comment 14

10 years ago
Something like this?
.table span { display: table; }

  <button onclick="document.getElementById('link13').className = (document.getElementById('link13').className == 'table') ? 'inline' : 'table';">Toggle inline/table for span child of link #13</button>
  <a id="link13" href="http://www.google.com" class="table"><span>Link #13</span></a>
</td>
Assuming that changing that class also changed the :before content on the link, sure.

Comment 16

10 years ago
I don't think we need to get too stressed about getting that one fixed for ff3.
We're short on time, and i don't think real web pages will use :before for ARIA widgets as long as IE doesn't support it.

After chatting with Scott Haeger on IRC, I'm removing orca:high from this.
Blocks: 343213, 376943
No longer blocks: 374212, 396346
Summary: Image frame changes causes recreation of ATK objects → Image frame changes via :before/:after causes recreation of accessible objects
Whiteboard: orca:high
Comment on attachment 286159 [details] [diff] [review]
If frame *type* doesn't actually change, don't fire notification to invalidate a11y tree

This looks wrong to me (it'll miss notifications).
Attachment #286159 - Flags: superreview?(bzbarsky)
Attachment #286159 - Flags: superreview-
Attachment #286159 - Flags: review?(bzbarsky)
Attachment #286159 - Flags: review-

Updated

9 years ago
Keywords: access
Mass un-assigning bugs assigned to Aaron.
Assignee: aaronleventhal → nobody

Updated

6 years ago
Depends on: 686400
Alexander, is this still valid after all the reorg happening for Firefox 4? I would suspect not, but would like you to take a look. Rich, can you still reproduce?

Comment 20

5 years ago
(In reply to Marco Zehe (:MarcoZ) from comment #19)
> Alexander, is this still valid after all the reorg happening for Firefox 4?
> I would suspect not, but would like you to take a look. Rich, can you still
> reproduce?

I bet it's still valid, we have more generic bug 686400 for problems like this
You need to log in before you can comment on or make changes to this bug.