1792120 - [CTW] Stale relative-bounds values on iframe doc when updating iframe padding

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Nathan LaPré]

To reproduce this issue,

0. Enable the accessibility cache in about:config.
1. Open this example page:

data:text/html,<iframe style='height:100px; width:100px; padding:100px' src='data:text/html,hello world'></iframe>

2. With a screen reader, examine the bounding boxes of the elements. In NVDA, you can right-click on the taskbar icon -> Preferences -> Settings -> Vision -> Highlight Navigator Object to enable drawing of bounding boxes. The bounding boxes should look correct.
3. Open the Firefox Inspector and select the "iframe" DOM node.
4. Remove the padding from the iframe's style by checking the checkbox next to the "padding: 100px;" line.
5. Examine the bounding boxes of elements again.

Expected behavior: The bounding boxes adjust to account for the change in padding.
Actual behavior: The bounding boxes (within the iframe) do not adjust to account for the change in padding. Instead, they retain their previous positions (that they had when the padding was present).

Note that padding might not be the only adjustment that causes problems. I suspect border, width, and height attributes (essentially, things that affect bounds) may also show similar behavior, but I primarily investigated via padding.

Tested on Firefox Nightly 106.0a1 on Windows 11.

Comment 1

[Nathan LaPré]

I investigated this issue a bit in an attempt to better understand what's going on. I dumped the entire contents of the cache for all accessibles in the tree before and after removing the padding in the above example to figure out what's stale. It looks like the culprit is the iframe doc's cached relative-bounds. LocalAccessible::BundleFieldsForCache doesn't get called on the iframe doc when you adjust padding on the iframe element. RemoteAccessibleBase::BoundsWithOffset relies on those cached relative-bounds to calculate its answer, so you end up with the outer doc accessible bounds being correct, but everything downstream of it being incorrect (due to the stale relative-bounds). Really, the relative-bounds are stale for the true OOP iframe case, too (I checked), it's just that the BoundsWithOffset calculation works a bit differently when there's a real process boundary, in such a way that the problem is masked.

It's possible that we need to monitor padding in MaybeQueueCacheUpdateForStyleChanges. My feeling is that, however it happens, we need to ensure that BundleFieldsForCache gets called on the iframe doc when certain bounds-affecting attributes of the iframe element are changed. I think that would cause us to recalculate the relative bounds and update the cache accordingly. Maybe that means queueing a cache update for the outer doc accessible's child in MaybeQueueCacheUpdateForStyleChanges if we see a change to {padding, border, width, height, maybe more?}. Alternately, maybe we need a notification from layout to update that iframe doc.

See more of this discussion (including thoughts from Morgan) here.

Comment 2

[James Teh [:Jamie]]

Attachment: Bug 1792120: Use the cached offset from GetUsedBorderAndPadding for all OuterDocAccessibles, not just OOP iframes.

Aside from making the code consistent for OOP and in-process iframes, this also fixes incorrect bounds in in-process iframe documents when the iframe's padding is changed.
We always pushed a cache update for the iframe when the padding was changed.
However, we previously relied on parent-relative bounds for in-process iframes, which didn't get updated when the iframe's padding changed.

Comment 3

[Marco Zehe (:MarcoZ)]

Should this be in the M4 milestone since you have a patch for it @Jamie?

Comment 4

[James Teh [:Jamie]]

I'm not necessarily marking things as ctw-m4 unless they really should block the m4 milestone. I didn't think this one (by itself) needed to block. That said, it turns out that this is a code dependency for a bug which does block, so it's probably reasonable to make it part of m4.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93e32ba1ad9f
Use the cached offset from GetUsedBorderAndPadding for all OuterDocAccessibles, not just OOP iframes. r=morgan

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/93e32ba1ad9f