Joanmarie Diggs Reporter
	Description • 18 years ago

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070222 Minefield/3.0a3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070222 Minefield/3.0a3pre

When you type some text into the entry on the Find toolbar and the string entered cannot be found on the page, a label displays "Phrase not found".
Backspacing over some of the characters entered so that there is a match causes this label's text to change/not be displayed.

Using at-poke to log events, it seems that no at-spi events are being generated for the appearance of/disappearance of/changes to this label.  at-poke event log to be attached.

Reproducible: Always

Steps to Reproduce:
1. Launch at-poke
2. Navigate to a web page
3. Press control f, type some text that eventually will not match.
4. Backspace over some of the text so that there is a match.
Actual Results:  
No at-spi events are generated with respect to the "Phrase not found" label.

Expected Results:  
at-spi events would be generated with respect to the "Phrase not found" label.

	Joanmarie Diggs Reporter
	Comment 1 • 18 years ago

This log is the result of typing "what" in the find entry, then backspacing over the text.  The string failed to match once the "a" had been typed.

	Aaron Leventhal Assignee
	Comment 2 • 18 years ago

For Linux, we should be firing children changed events, but if we aren't that's a more general bug. Can you check to see if we're getting them?

That text should really be in a container with xhtml:role="wairole:alert". That way when it gets displayed we will fire the EVENT_ALERT in MSAA.

We could also set the aaa:invalid="true" on the find entry itself once the text is not found. This would generate a state change event for that field.

	Joanmarie Diggs Reporter
	Comment 3 • 18 years ago

You are firing children changed events.  At least children-changed:add events. I just checked in the Add Bookmarks dialog, History Panel, View button within the History panel, and the Toolbars submenu of the View menu.

They are also fired for each character that gets added or removed from the Find toolbar's entry and -- I just noticed this because I forgot to turn off event logging -- each character that gets added or removed from text boxes such as this "Additional Comments" text box. :-)  Should they be fired for entries/text boxes?

Regardless, in my quick examination thus far, it seems that the events we get are all add events whose detail1 is -1, e.g.:

children changed event (object:children-changed:add) -1 0 parent: A|co|ac|te|ed:entry:: child: <Null>

I would expect some children-changed deleted events, but I haven't seen any yet.  Also, I *think* convention is for the detail1 to reflect the position of the child with respect to the parent, but I'm not positive about that. 

	Aaron Leventhal Assignee
	Comment 4 • 18 years ago

We shouldn't fire children-changed events for every character, just fire text-changed events for that. Sounds like an important bug to file. Does it happen with every entry field? Please file a bug.

If the legit children-changed events don't have the correct event details, that is yet another bug to be filed.

	Joanmarie Diggs Reporter
	Comment 5 • 18 years ago

Re the firing in entries, yes and done:  please see bug #371394.

I want to look closer at the legit events, compare them to similar events in other apps, etc. before filing that bug -- assuming it indeed needs filing -- but I'll do that some time today.

Thanks!!

	Aaron Leventhal Assignee
	Comment 6 • 18 years ago

See also bug 371273

	Ginn Chen
	Comment 7 • 17 years ago

"Phrase Not Found", "Reached end of page, continued from top", ""(empty string) is ONE label, its text changes accordingly.

One issue here is, when it changed to empty string, at-tools on Linux doesn't have the label name updated.
It is caused by http://lxr.mozilla.org/seamonkey/source/accessible/src/atk/nsAccessibleWrap.cpp#674

Since the label is always there, "children-changed" is not expected.
I'll try "wairole:alert"

	Ginn Chen
	Comment 8 • 17 years ago

wairole:alert is not really helpful, because the label is not hiding/showing, it is always there.
Should we use event.initEvent('NameChange', false, true); in script in this case?

	Aaron Leventhal Assignee
	Comment 9 • 17 years ago

I think we should wait for Surkov's fix to bug 371394, which will automatically generate text change events via nsDocAccessible::CharacterDataChanged()

	Ginn Chen
	Comment 10 • 17 years ago

FWIW: bug 371394 doesn't solve this bug automatically.  

	alexander :surkov (:asurkov)
	Comment 11 • 17 years ago

(In reply to comment #10)
> FWIW: bug 371394 doesn't solve this bug automatically.  
> 

Hierarchy is toolbar (Accessible) -> label (TextAccessible). The bug 371394 doesn't fix it because toolbar is not hypertext accessible and label doesn't contain textnode. So I think we should fix it because for UI it's a text change and therefore text changed event should be fired.

But why are show/hide events are not fired (nsDocAccessible::InvalidateCacheSubtree() isn't called)?

	Ginn Chen
	Comment 12 • 17 years ago

See #c7,

The label is always there, but the text changes from empty string to "Phase Not Found".
So I think there's no children changes.

	alexander :surkov (:asurkov)
	Comment 13 • 17 years ago

(In reply to comment #12)
> See #c7,
> 
> The label is always there,

yes, but @hidden attribute is appeared and we do not call InvalidateCacheSubtree().

> but the text changes from empty string to "Phase Not
> Found".
> So I think there's no children changes.

Yes, there is not DOM children changes because label hasn't them even when label shows some text.

	alexander :surkov (:asurkov)
	Comment 14 • 17 years ago

Question. Should an accessible that holds plain text only fire text changed events when its text was modified and implements nsIAccessibleText interface?

	Aaron Leventhal Assignee
	Comment 15 • 17 years ago

Anything with text should implement nsIAccessibleText.

The only time I could think of a text change where that's not true, is if the text was empty in the first place.

That might be what's going wrong, in which case it's fixed if we make the proposed change to nsHyperTextAccessible::QueryInterface(), where text/hypertext/editable text interfaces are always supported.

	Tim Keenan
	Comment 16 • 17 years ago

Is there anything new here?  Have the changes to nsHyperTextAccessible::QueryInterface(), been implemented?

	Aaron Leventhal Assignee
	Comment 17 • 17 years ago

I think Surkov was right when he said:
> yes, but @hidden attribute is appeared and we do not call
> InvalidateCacheSubtree().

The value attribute doesn't seem to appear as an anonymous child of the xul:label. Perhaps it's only in the frame tree.

This would explain why the xul:label has no text child and thus has no text. Perhaps it also helps explains why InvalidateCacheSubtree() isn't called.

	Aaron Leventhal Assignee
	Comment 18 • 17 years ago

Comment 17 is more crreoct. More details.

Related issue:

This is also probably an issue with <checkbox label=> etc. We probably don't implement the accessible text interface there.

Ideas for implementing a fix:

The XUL label is implemented with nsTextBoxFrame.cpp. There is a string called mTitle and mCroppedTitle. The PaintTitle() method is where the label is painted.

I can think of 4 ways to fix this:
1) Reimplement <label value> using an anonymous XBL child text node instead -- big risky change, not for FF3 that's for sure
2) Change nsHyperTextAccessible so it can get text from the accessible name of the label, similar to how list bullets currently get their text from the accessible name -- many methods such as character extents won't work
3) Implement nsIAccessibleText direclty on nsXULTextAccessible -- lots of work
4) Temporary fix just for this find bar only -- use <description control="textbox">foo</label> -- it will be treated as secondary text for the textbox. Because it has an anonymous child the text change events should work.

For Firefox 3, I prefer the temporary solution #4.

	Aaron Leventhal Assignee
	Comment 20 • 17 years ago

We can file a follow-up bug to make the following also work in the find bar:
- labelled-by relation for the label
- described-by relation on the description
- description change events when the description (find status) changes

	alexander :surkov (:asurkov)
	Comment 21 • 17 years ago

(In reply to comment #18)

> For Firefox 3, I prefer the temporary solution #4.
> 

I'm sure we can find another examples where XUL is used. It would be nice to fix real bug. Could you file following up bug?

	Aaron Leventhal Assignee
	Comment 22 • 17 years ago

Filed follow-up bugs:
bug 396165 for more fixes to the find bar specifically -- not sure if they're needed.
bug 396166 to fix the core issue with xul:label -- not sure if it's needed for FF3, because Orca falls back on the accessible name. It was an issue in this case because text change events were missing. The label doesn't usually change.

	Asaf Romano (gone)
	Comment 23 • 17 years ago

Comment on attachment 280780 [details] [diff] [review]
1) Use solution #4 from comment 18 for now, because it's simple & low risk and solves the problem, 2) It also ties accessible description property & relations so the status is read on textbox focus

or you could use node.textContent.

r=mano either way, "ew".

	Aaron Leventhal Assignee
	Comment 24 • 17 years ago

Comment on attachment 280780 [details] [diff] [review]
1) Use solution #4 from comment 18 for now, because it's simple & low risk and solves the problem, 2) It also ties accessible description property & relations so the status is read on textbox focus

Well, I would have to say "ew" about the fact that nsTextBoxFrame is needed, and that we don't just use anonymous content. That's the hack that needs to be fixed eventually IMO