1776275 - Adjust AUPDP used in BoundsInAppUnits for consistency (and correctness in BoundsInCSSPixels)

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Morgan Reschenberg [:morgan]]

bounds/browser_test_zoom_text.js is broken, we aren't getting scaled X/Y/Width/Height values after zooming.
I'm not sure why we're comparing getBounds(textNode) to getRangeExtents(0, -1) for the same text node, so that might be part of the problem.

In any case, either the text bounds/bounds code needs to be fixed, or this test (and its friends) need to be adjusted.

Comment 1

[Morgan Reschenberg [:morgan]]

The thing that's failing here is our BoundsInCSSPixels value isn't the same as our Bounds value divided by DPR. The DPR and original bounds value looks correct, so it seems like the problem is in BoundsInCSSPixels.
The only real difference between BoundsInCSSPixels and Bounds is the former calls into BoundsInAppUnits and then converts to CSS pixels.
Right now, we use a different app units conversion factor in BoundsInCSSPixels than we do in Bounds (we fetch the former from the top level browsing context, and the latter we retrieve from the cache of the top level document).

Making BoundsInCSSPixels use the cached value seems to fix this for remote browsers, but iframes and remote iframes are still wrong. I suspect I'm using some not-quite-top-level-doc but anyway, this is some information :)

Comment 2

[James Teh [:Jamie] (away returning 28 Jan)]

(In reply to Morgan Reschenberg [:morgan] from comment #0)

I'm not sure why we're comparing getBounds(textNode) to getRangeExtents(0, -1) for the same text node

As discussed on Zoom, this just ensures our text bounds code behaves correctly with respect to our Accessible bounds code. However, i agree that it probably makes sense to compare with DOM bounds as well, since the current test assumes that our Accessible bounds code actually works (which means it relies on the success of a separate test to prove that).

(In reply to Morgan Reschenberg [:morgan] from comment #1)

Making BoundsInCSSPixels use the cached value seems to fix this for remote browsers, but iframes and remote iframes are still wrong.

As discussed on Zoom, this is actually a bug in the way the test applies zoom, but we should still fix BoundsInCSSPixels anyway.

Comment 3

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1776275: Consistently fetch fullZoom from top level browsing context in tests, cache on the top level doc in core r?Jamie

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/365e262b3dad
Consistently fetch fullZoom from top level browsing context in tests, cache on the top level doc in core r=Jamie

Comment 5

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing mochitest failures on browser_test_zoom_text.js

Backout link
Push with failures
Link to failure log
Failure line :
TEST-UNEXPECTED-FAIL | accessible/tests/browser/hittest/browser_test_zoom_text.js | Wrong offset at given point (152, 126.5) for [DOM node id: paragraph, role: paragraph, address: 0x140d4340] - Got 10, expected 9
TEST-UNEXPECTED-FAIL | accessible/tests/browser/hittest/browser_test_zoom.js | Wrong direct child accessible at the point (27, 198) of [DOM node id: default-iframe-body-id, role: document, name: "Accessibility Fission Test", address: 0x205c9f7de50], sought paragraph - Got [xpconnect wrapped (nsIAccessible, nsIAccessibleDocument) @ 0x205c4842e80 (native @ 0x205c9f7de50)], expected [xpconnect wrapped nsIAccessible @ 0x205ce2da640 (native @ 0x205c

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0d66b6a68bb
Consistently fetch fullZoom from top level browsing context in tests, cache on the top level doc in core r=Jamie

Comment 7

[bszekely]

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