Closed Bug 1816022 Opened 2 years ago Closed 2 years ago

[CTW] Bounds incorrect for text within span, section, with <!DOCTYPE html>

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
112 Branch
Tracking Flags:
Tracking Status
firefox111 --- fixed
firefox112 --- fixed

People

(Reporter: nlapre, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

(Whiteboard: [ctw-m5])

Attachments

(1 file)

Bug 1816022: LocalAccessible::ParentRelativeBounds: If the bounding frame's rect is empty but its union rect is not, offset by its union rect.
(deleted), text/x-phabricator-request
dmeehan
: approval-mozilla-beta+
Details

STR:

  1. Load the following page in Stable and Nightly (111.0a1, Feb. 9), assuming the accessibility cache is enabled only in Nightly:
data:text/html,<!DOCTYPE html><html><span tabindex="-1"><div>Testing</div></span></html>
  1. Note the accessible bounds of the "Testing" text in each browser.
    Expected results: The bounds are the same.
    Actual results: With the cache on, the button accessible has greater y-coordinates. These coordinates do not visually bound the button.

This is a slimmed-down testcase of something I found on YouTube, while testing cached bounds of major websites. This example comes from the channel name displayed under a video.

Something that really confuses me about this: if I remove <!DOCTYPE html>, the problem goes away. The title of this bug should probably be changed.

Bizarre!

As I understand it, without <!doctype html>, the document is in quirks mode, which enables some legacy layout stuff. I'm not precisely sure of the details of said quirks, though.

The frame tree is subtly different with quirks mode vs standards mode. With standards mode (where we get the bad result):

            line@1f114bd03d8 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=0, h=0) ink-overflow=(x=0, y=-840, w=0, h=0) scr-overflow=(x=0, y=-840, w=0, h=1020) <
              Inline(span)(0)@1f114bcffd8 parent=1f114bcfec8 next=1f114bd0228 IBSplitSibling=1f114bd0228 (x=0, y=-840, w=0, h=1020) [content=1f115804500] [cs=1f114f172f8] <
              >
            >
            line@1f114bd0428 count=1 state=block,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=60480, h=1080) <
              Block(span)(0)@1f114bd0228 parent=1f114bcfec8 next=1f114bd0338 IBSplitSibling=1f114bd0338 IBSplitPrevSibling=1f114bcffd8 (x=0, y=0, w=60480, h=1080) [content=1f115804500] [cs=1f114f175c8:-moz-block-inside-inline-wrapper] <
                line@1f114bd02e8 count=1 state=block,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=60480, h=1080) <
                  Block(div)(0)@1f114bd0078 parent=1f114bd0228 (x=0, y=0, w=60480, h=1080) [content=1f115804580] [cs=1f114f173e8] <
                    line@1f114bd01d8 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=2813, h=1080) <
                      Text(0)"Testing"@1f114bd0138 parent=1f114bd0078 (x=0, y=30, w=2813, h=1020) [content=1f115804600] [cs=1f114f174d8:-moz-text] [run=1f114bddca0][0,7,T] 
                    >
                  >
                >
              >
            >
            line@1f114bd0478 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=1080, w=0, h=0) ink-overflow=(x=0, y=240, w=0, h=0) scr-overflow=(x=0, y=240, w=0, h=1020) <
              Inline(span)(0)@1f114bd0338 parent=1f114bcfec8 IBSplitPrevSibling=1f114bd0228 (x=0, y=240, w=0, h=1020) [content=1f115804500] [cs=1f114f172f8] <

With quirks mode (where we get the expected result):

            line@1f114b073d8 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=0, h=0) <
              Inline(span)(0)@1f114b06fd8 parent=1f114b06ec8 next=1f114b07228 IBSplitSibling=1f114b07228 (x=0, y=0, w=0, h=0) [content=1f114d03500] [cs=1f114bbc208] <
              >
            >
            line@1f114b07428 count=1 state=block,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=60480, h=1080) <
              Block(span)(0)@1f114b07228 parent=1f114b06ec8 next=1f114b07338 IBSplitSibling=1f114b07338 IBSplitPrevSibling=1f114b06fd8 (x=0, y=0, w=60480, h=1080) [content=1f114d03500] [cs=1f114bbc4d8:-moz-block-inside-inline-wrapper] <
                line@1f114b072e8 count=1 state=block,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=60480, h=1080) <
                  Block(div)(0)@1f114b07078 parent=1f114b07228 (x=0, y=0, w=60480, h=1080) [content=1f114d03580] [cs=1f114bbc2f8] <
                    line@1f114b071d8 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=2813, h=1080) <
                      Text(0)"Testing"@1f114b07138 parent=1f114b07078 (x=0, y=30, w=2813, h=1020) [content=1f114d03600] [cs=1f114bbc3e8:-moz-text] [run=1f114bc00d0][0,7,T] 
                    >
                  >
                >
              >
            >
            line@1f114b07478 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=1080, w=0, h=0) <
              Inline(span)(0)@1f114b07338 parent=1f114b06ec8 IBSplitPrevSibling=1f114b07228 (x=0, y=1080, w=0, h=0) [content=1f114d03500] [cs=1f114bbc208] <
  1. Our bounds for the div are 14 pixels (840 app units) too large with standards mode.
  2. The first span frame has y=-840 h=1020 in standards mode, y=0 h=0 in quirks mode.
  3. The third span frame has y=240 in standards mode, y=1080 in quirks mode.

Edit: I flipped the results around, so had to correct them. 😳

The key is that first span frame. It has y=-840, but when we call GetAllInFlowRectsUnion, it returns a rect which starts +840 below that. TransformRect is based on frame position (the -840) but our bounds are based on GetAllInFlowRectsUnion, so we end up with a value which is 840 too large.

GetAllInFlowRectsUnion normally doesn't return something which excludes part of the frame's rect. I guess this is different because the span's primary frame has y=-840, but it also has a width of 0, so I guess it doesn't make sense to include it.

I guess we need to find the first frame with a non-0 rect and calculate relative to that. That feels pretty yucky, though, so I'll keep thinking on it.

Assignee: nobody → jteh
Severity: -- → S3

For the record, I originally worked on a branch locally which calculates bounds relative to the nearest scrollable Accessible rather than the parent Accessible. This means we don't have to worry about ib-splits (DOM nodes having multiple frames in different subtrees), continuations crossing subtrees, etc. because a scrollable frame is always an ancestor. Also, there probably? aren't that many cases where a parent moves and the direct children stay in the same position relative to the parent.

This worked pretty well, except for one problem. When you set position: fixed on a container, we correctly update the cache for the container. However, it seems that the container doesn't necessarily get reflowed and the children don't get SetPosition notifications. The practical upshot is that the bounds of the children are stale; they still include the scroll position.

In the end, I went with the solution attached here. However, we may want to consider going with the scrollable ancestor solution if we encounter more problems like this. We should be able to find a solution for the fixed problem above; e.g. push cache updates for bounds of direct children when position: fixed is added.

(In reply to James Teh [:Jamie] from comment #3)

The key is that first span frame. It has y=-840, but when we call GetAllInFlowRectsUnion, it returns a rect which starts +840 below that. TransformRect is based on frame position (the -840) but our bounds are based on GetAllInFlowRectsUnion, so we end up with a value which is 840 too large.

GetAllInFlowRectsUnion normally doesn't return something which excludes part of the frame's rect. I guess this is different because the span's primary frame has y=-840, but it also has a width of 0, so I guess it doesn't make sense to include it.

I guess we need to find the first frame with a non-0 rect and calculate relative to that. That feels pretty yucky, though, so I'll keep thinking on it.

Where does that -840 come from...? I'm not sure I understand why the trees are different (apart from the fact that one of them is quirks and one isn't).

Flags: needinfo?(jteh)

As discussed on Zoom, there are slight differences in rendering between quirks mode and standards mode. Digging a bit more, my guess is that this line-height 0 quirk means the first line gets a height of 0 in quirks mode, which worked better for us with the old code. In contrast, without the line-height quirk, the line-height can't be 0, which means the first line has to have a height of 840. Its y must thus be offset accordingly because that line has no actual text.

But honestly, I don't know. I just work here.

Flags: needinfo?(jteh)
Pushed by jteh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4c255157e033 LocalAccessible::ParentRelativeBounds: If the bounding frame's rect is empty but its union rect is not, offset by its union rect. r=morgan

https://hg.mozilla.org/mozilla-central/rev/4c255157e033

Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox112: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 112 Branch

Comment on attachment 9316999 [details]
Bug 1816022: LocalAccessible::ParentRelativeBounds: If the bounding frame's rect is empty but its union rect is not, offset by its union rect.

Beta/Release Uplift Approval Request

  • User impact if declined: Screen readers won't report content under the mouse correctly in some cases when the accessibility cache is enabled via a release experiment.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Covered by tests. Only impacts screen coordinates reported to accessibility clients when the accessibility cache is enabled.
  • String changes made/needed:
  • Is Android affected?: No
Attachment #9316999 - Flags: approval-mozilla-beta?

Comment on attachment 9316999 [details]
Bug 1816022: LocalAccessible::ParentRelativeBounds: If the bounding frame's rect is empty but its union rect is not, offset by its union rect.

Approved for 111.0b5

Attachment #9316999 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: