438784 - URL of loaded page no longer exposed as the accValue for the Doc accessible.

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Marco Zehe (:MarcoZ)]

Regression range is
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-03-29+00:00&maxdate=2008-03-30+05:00&cvsroot=/cvsroot.

Regression introduced after beta 5. This causes the NVDA developers not to be able to look at a document's URL without resorting to the iSimple* interfaces to determine if a link is a same page link. Using IAccessible2 should not under any circumstances require the use of any iSimple* interfaces.

Alexander, any obvious candidates from that day sticking out at you?

Comment 1

[Marco Zehe (:MarcoZ)]

It's broken in the platform independent code, not any MSAA specific stuff.

Comment 2

[Aaron Leventhal]

Only MSAA needs to do expose the doc's URL in the value. In ATK/AT-SPI it's exposed as a document attribute.

Comment 3

[Marco Zehe (:MarcoZ)]

Regression from bug 425693.

Comment 4

[Marco Zehe (:MarcoZ)]

Attachment: Only allocate memory if the value is not empty, but return S_OK in any case.

This restores functionality. With nsDocAccessibleWrap::get_accValue being the only instance overriding nsAccessibleWrap::get_accValue, and from tests, I don't see any negative impact from this. Textfields, comboboxes still work as expected.

Comment 5

[Aaron Leventhal]

Comment on attachment 324761 [details] [diff] [review]
Only allocate memory if the value is not empty, but return S_OK in any case.

r+ for trunk.

Comment 6

[Marco Zehe (:MarcoZ)]

Try-Server builds will appear here:
1213269449/

Jamie, please check it out and see if this is what you want, and if you see any negative side effects with NVDA.

Comment 7

[Marco Zehe (:MarcoZ)]

Attachment: Alternative: Revert part of fix for bug 425693.

This also fixes the problem and reinstates part of the backout I did in bug 425693 from bug 421650. Reminder: The original reason to do this for the MSAA-related methods was bug 424971, which was fixed by backing out the "if isempty return false" part for get_accname, to fix a problem JAWS 8 was having with this in alerts.

Comment 8

[alexander :surkov (:asurkov)]

Marco, if your alternative way doesn't bring any regression for Jaws8 then I like it more. Just please add comment for get_accName why it's a different. Also what do you think about another methods?

Comment 9

[Marco Zehe (:MarcoZ)]

(In reply to comment #8)
> Also what do you think about another methods?

You mean the other methods that were changed in bug 425693? These changes didn't introduce any regressions AFAIK, the only one that regressed was this one.

Comment 10

[Marco Zehe (:MarcoZ)]

Comment on attachment 324787 [details] [diff] [review]
Alternative: Revert part of fix for bug 425693.

Officially asking review for the alternative and more safe patch. I'll add a comment to get_accName to clarify why that one behaves differently.

Comment 11

[alexander :surkov (:asurkov)]

Comment on attachment 324787 [details] [diff] [review]
Alternative: Revert part of fix for bug 425693.

r=me

Comment 12

[Marco Zehe (:MarcoZ)]

Pushed to mozilla-central in changeset:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/0e4035019607

Comment 13

[Marco Zehe (:MarcoZ)]

When we get to platform-specific tests, revisit this bug to add a testcase.

Comment 14

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008062603 Minefield/3.1a1pre

Comment 15

[Marco Zehe (:MarcoZ)]

Comment on attachment 324787 [details] [diff] [review]
Alternative: Revert part of fix for bug 425693.

Has been baking on 1.9.1a1 for weeks, didn't cause any regressions, and would help the NVDA people tremendously with identifying links that point to a spot within the same page.

Comment 16

[Samuel Sidler (old account; do not CC)]

Comment on attachment 324787 [details] [diff] [review]
Alternative: Revert part of fix for bug 425693.

Approved for 1.9.0.2. Please land in CVS. a=ss.

Comment 17

[Marco Zehe (:MarcoZ)]

Checking in accessible/src/msaa/nsAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/msaa/nsAccessibleWrap.cpp,v  <--  nsAccessibleWrap.cpp
new revision: 1.119; previous revision: 1.118
done