James Teh [:Jamie] Reporter
	Description • 9 years ago

Str:
1. Perform a Google Search; e.g. foo bar.
2. Get the coordinates for the centre screen point of the link for the first search result.
3. From the top level accessible, hit test with these coordinates. Hit test down the tree until you can't go any further.
Actual: The accessible is a (fairly distant) ancestor of the link.
Expected: The accessible should be the link.

Hit testing is stopping way too early for some reason. The practical upshot is that mouse tracking is quite broken in NVDA. This is quite problematic for users with partial sight and developers.

This does not occur with all pages. However, we've had reports that it occurs elsewhere, though I don't have any specific URLs. I've no idea why it occurs on some pages and not others.

Worked fine in Firefox 34, but broken in Firefox 35.

NVDA issue ticket: http://community.nvda-project.org/ticket/4825

	alexander :surkov (:asurkov) Assignee
	Comment 1 • 9 years ago

It'd be good to have a regression range, I doubt that a11y changes were guilty in that. Is there anybody willing to help with regression finding?

	Marco Zehe (:MarcoZ)
	Comment 2 • 9 years ago

First broken build is the 2014-08-23 build. Regression range is: 
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=0b9dd32d1e16&tochange=64c4ec2df3d4

	Marco Zehe (:MarcoZ)
	Comment 3 • 9 years ago

The only non-GAIA accessibility-related landing in that period is bug 1047696.

	Marco Zehe (:MarcoZ)
	Comment 4 • 9 years ago

OK, scrap those last two comments. I was chasing a ghost that finally turned on me with the middle finger.

Of course, the correct regression range has to be in the Firefox 35 cycle, not the 34 one, if the 35 release shows the problem. And it is!

Last good build is 2014-09-16, first bad build is 2014-09-17. Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3b7921328fc1&tochange=8252eae8278c

	alexander :surkov (:asurkov) Assignee
	Comment 5 • 9 years ago

there's a div having zero width, the alg stops on it

just in case DOMi script to show all findings:

var links = [];
function findLink(a)
{
  if (tree.mAccService.getStringRole(a.role) == "link" &&
      a.name && a.name.indexOf("Foobar - Wiki") != -1) {
    links.push(a);
  }
  return false;
}
tree.clearSearch();
tree.search(accessible, findLink);

var xObj = {}, yObj = {}, wObj = {}, hObj = {};
links[0].getBounds(xObj, yObj, wObj, hObj);

var x = xObj.value + wObj.value / 2;
var y = yObj.value + hObj.value / 2;

var acc = accessible.getChildAtPoint(x, y);
while (acc) {
  tree.search(accessible, function(a) { return acc == a });
  acc = acc.getChildAtPoint(x, y);
}

	alexander :surkov (:asurkov) Assignee
	Comment 6 • 9 years ago

(In reply to Marco Zehe (:MarcoZ) from comment #4)

> Last good build is 2014-09-16, first bad build is 2014-09-17. Regression
> range:
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=3b7921328fc1&tochange=8252eae8278c

Is there a chance to find exact regression? It appears we regressed because of layout stuff, a11y code simply calls nsLayoutUtils::GetAllInFlowRectsUnion to find boundaries (note, Element.offsetWidth also returns 0).

	Marco Zehe (:MarcoZ)
	Comment 7 • 9 years ago

(In reply to alexander :surkov from comment #6)
> Is there a chance to find exact regression? It appears we regressed because
> of layout stuff, a11y code simply calls
> nsLayoutUtils::GetAllInFlowRectsUnion to find boundaries (note,
> Element.offsetWidth also returns 0).

Result from hg bisect session:
The first bad revision is:
changeset:   205509:627909e0506e
user:        Alexander Surkov <surkov.alexander@gmail.com>
date:        Tue Sep 16 13:30:23 2014 -0400
summary:     Bug 1064877 - dexpcomify Accessible class, r=tbsaunde

My bisection also shows that I am building with one changeset before this one along the way, which is still good, so this is definitely confirmed to be the bad revision that introduces the problem.

	Marco Zehe (:MarcoZ)
	Comment 9 • 9 years ago

FWIW, this patch does not appear to solve the problem. With this patch applied, I no longer hear NVDA's error sound, but the whole document area no longer speaks any content when mousing over it. NVDA normally speaks the element under the mouse, but now is completely silent.

	alexander :surkov (:asurkov) Assignee
	Comment 10 • 9 years ago

Comment on attachment 8552511 [details] [diff] [review]
patch

canceling per comment #9. I'd be great to have more input what can be wrong.

	Marco Zehe (:MarcoZ)
	Comment 11 • 9 years ago

Before this patch, mousing over the document area while NVDA was running would give constant error sounds (descending piano tone). Now, NVDA just sees nothing, but also doesn't give an error message any more. It feels like the whole document area is an empty area for hit testing.

	alexander :surkov (:asurkov) Assignee
	Comment 12 • 9 years ago

(In reply to Marco Zehe (:MarcoZ) from comment #11)
>  It feels like
> the whole document area is an empty area for hit testing.

it's strange since I was able to find that link in DOMi following Jamie's steps. So that has to be Windows or NVDA specific.

	alexander :surkov (:asurkov) Assignee
	Comment 13 • 9 years ago

just finished a build on windows and it works fine. Can it be something wrong with your build, Marco?

	Marco Zehe (:MarcoZ)
	Comment 14 • 9 years ago

Definitely confirmed with a new fresh build with today's code and your patch applied. The whole document area is blank to NVDA's mouse movement.

Can you kick off a try-server build and post the link here so both jamie and I can try that one?

	alexander :surkov (:asurkov) Assignee
	Comment 15 • 9 years ago

(In reply to Marco Zehe (:MarcoZ) from comment #14)
> Definitely confirmed with a new fresh build with today's code and your patch
> applied. The whole document area is blank to NVDA's mouse movement.

and it works without patch, right?

> Can you kick off a try-server build and post the link here so both jamie and
> I can try that one?

here's the previous one I made https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/surkov.alexander@gmail.com-39a410f1db85/

	Marco Zehe (:MarcoZ)
	Comment 16 • 9 years ago

Comment on attachment 8552511 [details] [diff] [review]
patch

OK, with the try-server build, mousing over document content works again. So I'm fine with it. :) Don't know why my local build shows such quirky behavior. Let's ignore that and proceed with the review.

	Yura Zenevich [:yzen] (please use "needinfo")
	Comment 17 • 9 years ago

Comment on attachment 8552511 [details] [diff] [review]
patch

Review of attachment 8552511 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good thank you!

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/ec1efe1d6f42

	James Teh [:Jamie] Reporter
	Comment 19 • 9 years ago

Thanks!

Marco/Alex, any chance of a backport? This is pretty serious for partially sighted users.

	alexander :surkov (:asurkov) Assignee
	Comment 20 • 9 years ago

Comment on attachment 8552511 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1064877
[User impact if declined]: broken mouse reading mode in NVDA
[Describe test coverage new/current, TreeHerder]: mochitest-a11y
[Risks and why]: no risk, partial backout
[String/UUID change made/needed]:no

	Marco Zehe (:MarcoZ)
	Comment 21 • 9 years ago

Comment on attachment 8552511 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1064877
[User impact if declined]: broken mouse reading in NVDA, possibly magnifiers affected, too
[Describe test coverage new/current, TreeHerder]: mochitest-a11y
[Risks and why]: No risk, partial backout
[String/UUID change made/needed]: No.

	Ryan VanderMeulen [:RyanVM]
	Comment 22 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/8af51e438e0e
https://hg.mozilla.org/releases/mozilla-beta/rev/e63d5e1865db