346833 - :before and :after text no longer exposed

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

Text in the frame tree that is rendered needs to be exposed to screen readers, no matter what.

:before and :after text used to be exposed, now it is not.

In any case it is static text and needs to be exposed with static=true.

Comment 1

[Aaron Leventhal]

Attachment: Walk frame tree for block/inline/inlineblock frames to make sure we catch :before and :after content

Comment 2

[Aaron Leventhal]

Attachment: Testcase

Comment 3

[Aaron Leventhal]

Eventually list bullets and :before/:after text will get a text attribute
"static:true". That's bug 340670.

In addition, we need to deal with all of the GetText[At|Before|After]Offset
implications when static text is part of a paragraph. Filed bug 348907 for that and any other nsHyperTextAccessible issues we'll need to handle.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I don't understand this patch.  Is there a reason that we don't handle :before and :after for positioned inlines, table captions, table cells, HTML <button>s, <select>, <legend>, <fieldset>, MathML, SVG?

If you actually want to test whether the frame is a container, just check !frame->IsLeaf().  If you want to test something else, the comment probably needs changing.

Comment 5

[Aaron Leventhal]

I backed out this fix because it caused bug 349533. Need to address bz's concerns as well.

Comment 6

[Aaron Leventhal]

Attachment: Try again: 1) Address hang - get nextsibling until content changes, (text continuation not used for separate accessible), 2) Always walk frames in HTML, 3) simplify list's CacheChildren()

Comment 7

[Evan Yan]

the patch caused a regression described on bug 349860.
reopen.

Comment 8

[Aaron Leventhal]

This also regressed bug 352150!

For <span>x<br>y</span>, no accessible object is created for "y"

Comment 9

[Aaron Leventhal]

Attachment: Walk all the next siblings

This fixes bug 352150. Evan, give this a try for bug 349860 as well. 

Dbaron wants us to walk the DOM tree because of Aural CSS. Since that's not really used on the web today, I'd prefer to leave that for the future. Walking the frame tree lets us continue our trunk work and makes it easy to get list bullet text, :before and :after text. Also will make it easier to get scrollbars.

Here's dbaron's comment:
you need to ask only for those display types that support :before and :after (which roughly means non-replaced elements)

Dbaron wrote:
> I suppose it depends on what you're trying to do but it moves you 
> significantly farther away from being able to do something like aural CSS
In response to my comment that we don't expose display:none elements, he said:
> What if it's @media screen { display: none; } ?

I think it's okay that we follow this approach for now

Comment 10

[Evan Yan]

Comment on attachment 238525 [details] [diff] [review]
Walk all the next siblings

it doesn't work for bug 349860. 
we need to do something in the "if" statement.

281     if (mState.frame && mState.siblingIndex < 0) {
       // need do something here

because we removed " && mState.frame->GetContent()->IsNativeAnonymous()" from the "if" condition.
see
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/accessible/src/base&command=DIFF_FRAMESET&file=nsAccessibleTreeWalker.cpp&rev1=1.18&rev2=1.19&root=/cvsroot

Comment 11

[Aaron Leventhal]

Comment on attachment 238525 [details] [diff] [review]
Walk all the next siblings

Evan, what you're doing to the if() statement is making it so that frames are not walked. I'm trying to fix both this bug and the regression by making it walk frames correctly. Changing that if() isn't the way to do that, it just makes it not walk the frames for the problem scenarios.

Comment 12

[Aaron Leventhal]

I have put fixes in both regressions: bug 349860 and bug 352150

We can close this bug again after those are marked fixed.

Comment 13

[Aaron Leventhal]

No need to reopen this. We didn't break it via any backouts -- we found other fixes.

Comment 14

[Ginn Chen]

need a fix without walking frames

Comment 15

[alexander :surkov (:asurkov)]

Do you want to create accessibles for :begin and :after? So

<style>
p:begin, p:after {
  content: text
}
</style>

<p>hello</p>

has accessible tree:

hypertext acc (p)
  text acc (:before)
  text acc (text content of p)
  text acc (:after)

right?

Comment 16

[Aaron Leventhal]

Alex, that was the plan.

Realistically, there is no point in fixing this until IE gets :before and :after. Until then authors won't use that.

Comment 17

[alexander :surkov (:asurkov)]

(In reply to comment #16)
> Alex, that was the plan.

Ok.

> Realistically, there is no point in fixing this until IE gets :before and
> :after. Until then authors won't use that.
> 

It's still important if we care about XUL. Though probably not hight priority.

Comment 18

[David Bolter [:davidb] (NeedInfo me for attention)]

Mass un-assigning bugs assigned to Aaron.

Comment 19

[alexander :surkov (:asurkov)]

Fixed on Gecko 2.0 by bug 530081, covered by mochitest - http://mxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/tree/test_gencontent.html?force=1

Comment 20

[Marco Zehe (:MarcoZ)]

Alex, are you sure this is fixed completely? I don't see the "enter a number" and the other text that are declared on input as :before and :after. Am I missing something?

Comment 21

[alexander :surkov (:asurkov)]

(In reply to comment #20)
> Alex, are you sure this is fixed completely? I don't see the "enter a number"
> and the other text that are declared on input as :before and :after. Am I
> missing something?

There's no generated content for input visually, iirc it was a bug but I failed to find it. Though once it's fixed then possibly we should tweak a11y support to expose it to AT (because we prune children for inputs).

Comment 22

[Marco Zehe (:MarcoZ)]

All right, then this is verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0) Gecko/20100101 Firefox/4.0