1815153 - [CTW] Bounds incorrect for scrolled position: sticky elements

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[James Teh [:Jamie]]

Similar to bug 1809836, but we decided to handle this separately.

STR:

1. Open this test case:

   data:text/html,<div style="position: sticky; top: 0px;"><button>top</button></div><script> for (let i = 0; i < 1000; ++i) { const div = document.createElement("div"); div.innerHTML = `<button>${i}</button>`; document.body.append(div); } </script>
   

2. Get the Accessible for the "top" button.
3. Get its bounds.
4. Scroll the page to the bottom.
5. Get the bounds for the "top" Accessible.
   • Expected: Same as step 3.

What's interesting is that on current mozilla-central, this actually works as expected! That surprised me, so I did a bit of debugging. It turns out that the cached relative-bounds for the div get updated every time you scroll. The y offset gets larger and larger, compensating for the increasing scroll offset. I'm not sure if this is due to the original reflow notification or the position change notification I added recently in bug 1812208.

Comment 1

[James Teh [:Jamie]]

Morgan, I forget some of the details of our conversation about this, but I vaguely recall this is basically the "naive approach" we discussed... except that it seems we already get the required notifications without any change at all. 😲 Surely it can't be this simple? Of course, we'll need to see whether this turns out to be a performance problem with regular bounds cache updates.

Nathan, I'd be interested to know whether any of the position: sticky failures you've seen in the wild are magically working now. Note that this will still probably break if the position: sticky element doesn't have an Accessible (bug 1814800 but for sticky), so if it doesn't, you might need to force one by adding, say, role="group" before you scroll.

Comment 2

[James Teh [:Jamie]]

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

I'm not sure if this is due to the original reflow notification or the position change notification

I verified that it's due to the position change notification, not reflow.

Comment 3

[James Teh [:Jamie]]

Test case:

data:text/html,<div><p>1</p><p style="position: sticky; top: 0px;">sticky</p><hr style="height: 100vh;"><p>2</p>

Before you scroll, sticky's bounds are correctly below the "1". If you scroll to the bottom of the document, sticky's bounds now report the top of the document.

Comment 4

[Nathan LaPré]

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

Nathan, I'd be interested to know whether any of the position: sticky failures you've seen in the wild are magically working now. Note that this will still probably break if the position: sticky element doesn't have an Accessible (bug 1814800 but for sticky), so if it doesn't, you might need to force one by adding, say, role="group" before you scroll.

Most of these issues in the wild have been position: fixed. I've found one major position: sticky issue in the wild which I've been struggling to reproduce for a while now: the Twitter tablist that holds "For you," "Trending", "News," "Sports," and so on. I've been unable to get the Testcase Reducer extension to get me a good isolated test of the position: sticky issue on Twitter, despite that being reasonably straightforward for position: fixed. That said, if I add role="group" to the parent div that has position: sticky for that Twitter tablist, the bounds issue is fixed. Do we want a separate bug for that? I think it is probably the same thing as what you've described here, so I doubt we need one yet.

Also, the test case you provided in comment #3 works for me in latest Nightly.

Comment 5

[James Teh [:Jamie]]

(In reply to Nathan LaPré from comment #4)

Most of these issues in the wild have been position: fixed.

I assume those are mostly fixed now apart from those that are still impacted by bug 1814800?

That said, if I add role="group" to the parent div that has position: sticky for that Twitter tablist, the bounds issue is fixed. Do we want a separate bug for that?

No; I'll expand bug 1814800 to cover it. Thanks for confirming.

Comment 6

[James Teh [:Jamie]]

(Removed accidental comment.)

Comment 7

[Nathan LaPré]

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

(In reply to Nathan LaPré from comment #4)

Most of these issues in the wild have been position: fixed.

I assume those are mostly fixed now apart from those that are still impacted by bug 1814800?

Yup. In particular, the LinkedIn and Google search menu bars are now fixed after scrolling. The Twitter bottom bar, with "Log In" and "Sign Up" is still broken due to the display: flex issue.

Comment 8

[James Teh [:Jamie]]

Attachment: Bug 1815153: Add test for cached a11y bounds of position: sticky.

Comment 9

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1815153: Add test to verify position:sticky bounds when scrolling, after dynamic style change r?Jamie

Comment 10

[Morgan Reschenberg [:morgan]]

Attachment: WIP: Bug 1815153: Sticky positioning support

Comment 11

[Morgan Reschenberg [:morgan]]

adding the patch I had locally for this, in case it becomes relevant later. also phab'd the test case I have, since it includes a test of style mutation which I think we should have in central. I'm wondering if all the position:fixed work from earlier is necessary now, or if that's covered by the position change notification too. I'll check

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1495a5008806
Add test to verify position:sticky bounds when scrolling, after dynamic style change r=Jamie

Comment 13

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/1495a5008806