Closed Bug 1776275 Opened 3 months ago Closed 3 months ago

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

Categories

(Core :: Disability Access APIs, defect)

defect

Tracking

()

RESOLVED FIXED
104 Branch
Tracking Status
firefox104 --- fixed

People

(Reporter: morgan, Assigned: morgan)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.

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 :)

Summary: Fix /bounds/browser_test_zoom_text.js → Adjust AUPDP used in BoundsInAppUnits for consistency (and correctness in BoundsInCSSPixels)

(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.

Assignee: nobody → mreschenberg
Status: NEW → ASSIGNED
Severity: -- → S4
Pushed by mreschenberg@mozilla.com:
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

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

Pushed by mreschenberg@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 104 Branch
You need to log in before you can comment on or make changes to this bug.