Closed Bug 1649239 Opened 5 years ago Closed 2 years ago

Unable to activate buttons on icloud.com with NVDA

Categories

(Core :: Disability Access APIs, defect, P3)

Product:
Core
Component:
Disability Access APIs
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: Jamie, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: papercut)

Attachments

(1 obsolete file)

STR (with the NVDA screen reader):

  1. Go to icloud.com.
  2. Log in with an Apple account.
  3. Press control+home to move to the top of the document.
  4. Press b twice to move to the Mail button.
  5. Press enter to activate it.
    • Expected: The Mail screen should load.
    • Actual: Nothing happens.

This occurs for any of the buttons to open the various apps; Mail, Calendar, Contacts, etc. None of them can be activated.

Findings:

  1. DoAction doesn't do anything.
  2. The buttons themselves have a width and height of 0 as exposed by Firefox a11y. The element's clientWidth/clientHeight/getBoundingClientRect also report 0 in the console, both in Firefox and Chrome. However, in Chrome, a11y reports a positive width and height for the buttons.
  3. These buttons do have CSS transforms, so it's possible bug 1472125 or bug 1613626 are relevant here.
  4. The Account Settings button is similarly broken, but it's slightly different: it does have a positive width and height, but IAccessibleText reports incorrect offsets when hit testing. That sounds very much like bug 1472125 .

Impact: This makes it impossible for NVDA users to use icloud.com with Firefox.
Assigning s3 because even though this is pretty severe for iCloud, I haven't seen this kind of breakage on any other sites in the wild. That said, iCloud is popular enough that we may want to re-evaluate this.
Originally reported as NVDA issue: https://github.com/nvaccess/nvda/issues/11307

I only tested this with NVDA, but it's highly likely this affects other a11y tools too, even on different platforms.

FWIW, this works as expected in Safari and VoiceOver on the Mac. haven't tried any other browser on MacOS, though.

Blocks: boundsa11y
Keywords: papercut

Distilled test case:
data:text/html,<div role="button" style="text-align: center; transform: matrix(1, 0, 0, 1, 292.5, 164.583); inline-size:0; block-size:0;">hello world</div>

I think this has to do with how/when we apply those i-size/b-size values in relation to the transform. Like Marco's comment above -- this example "works" in Safari insofar as they calculate a position, height, and width correctly, while we calculate 0 for all of those properties.

Interestingly, in the icloud example, Safari actually incorrectly calculates the position, though it is nonzero. When you use VO to focus the button, the actions are correctly passed, and the attributes of the button are read, but the VO cursor moves to the bottom of the screen, nowhere near the button visually. If I tile my Safari window to 1/2 of my screen and cursor through the buttons, the VO cursor moves the full length of my screen, outside the Safari window 😮

Okay here's some maybe-useful info/observations.

Safari:

  • button renders, VO cursor focuses properly (visually on top of the button)
  • Position reported as (x, y) = (300, 249) in CSS pixels presumably
  • Width, height are 38 and 37 respectively

Chrome:

  • Button renders, VO cursor focuses properly
  • Position reported as (300, 276)
  • W, H are 38 and 37

FF Nightly:

  • Button renders, when "focused", VO cursor is thrown to the outer web area, VO still reads "button, hello world"
  • Position, W, H all reported as 0

However! I dumped the frame tree and found something interesting -- our scrollable and ink overflows seem to match what safari and chrome report.
Ink (visual) overflow:

  • Position: (292, 164) -- there's some decimals here, but I assume Safari/Chrome are rounding when I use a11y inspector
  • W, H are 38 and 38

Scrollable overflow:

  • Position (292, 164)
  • W, H are 37 and 38 -- I double checked here and this is opposite what safari and chrome report for some reason? (ie. our width is their height, ..)

So I guess the question is, does it make sense to alter how we report position, size info (which I assume is through Bounds() ?) to report something like ink overflow instead of inherent frame size for some subset of accessibles?
This same issue (VO cursor thrown to webarea which I assume is a position issue, bad height, width) reproduces with this test case with no transform also, leading me to believe we do something wrong when layout sends us a 0x0 frame?
data:text/html,<div type="submit" role="button" style="text-align: center; inline-size:0; block-size:0;">hello world</div>

Flags: needinfo?(jteh)

As a temp fix:

  • I modified LocalAccessible::Bounds() to serve the rect from frame->InkOverflowRectRelativeToSelf() if the default (existing) getter gives us a frame with no area. This is probably too broad of a change, but I was using it to test if it fixes the position/height/width, which it does (Note: There's a transform there too, since the ink overflow rect we fetch is relative to self, not aBoundingFrame)
*aBoundingFrame = nsLayoutUtils::GetContainingBlockForClientRect(frame);
    nsRect unionRect = nsLayoutUtils::GetAllInFlowRectsUnion(
        frame, *aBoundingFrame, nsLayoutUtils::RECTS_ACCOUNT_FOR_TRANSFORMS);
    if (unionRect.IsZeroArea()) {
      nsRect overflow = frame->InkOverflowRectRelativeToSelf();
// Note: the ink overflow rect already combines overflow from the things in-flow, so we don't need to aggregate here like above
      nsLayoutUtils::TransformRect(frame, *aBoundingFrame, overflow);
      return overflow;
    }

    return unionRect;
  • This doesn't fix the action thing, and (at this point) it shouldn't -- DoCommand() uses the frame to calculate where to dispatch the mouse event, and because our frame is still position (0, 0) w/h 0/0, the event is still incorrectly dispatched. This feels like another thing that should be adjusted, but again -- not sure about the implications of using ink overflow here.

Thanks for looking into this. The unfortunate reality is that I have absolutely no clue when it comes to most layout concepts, so I don't really know how to answer this well...

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

However! I dumped the frame tree and found something interesting -- our scrollable and ink overflows seem to match what safari and chrome report.

What are scrollable and ink overflows? :) I kinda understand the concept of CSS overflow allowing things to flow outside their box, but I imagined that this would change the layout frame size accordingly. Is this not the case; is overflow actually drawn outside the frame? I thought everything that got drawn had to have a frame... but if so, where is the frame for this overflow stuff? Are there separate frames for overflow or something?

So I guess the question is, does it make sense to alter how we report position, size info (which I assume is through Bounds() ?) to report something like ink overflow instead of inherent frame size for some subset of accessibles?

I fear my understanding is too limited to know what impact this would have. On the other hand, what we're doing right now is clearly broken somehow, so I think everything is on the table at this point. :)

I guess one question is: if ink/scrollable overflow can report bounds outside a 0-sized frame, I assume they can also report bounds outside of a non-0-sized frame? If so, should we always be taking ink/scrollable overflow into account... somehow?

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

*aBoundingFrame = nsLayoutUtils::GetContainingBlockForClientRect(frame);
    nsRect unionRect = nsLayoutUtils::GetAllInFlowRectsUnion(
        frame, *aBoundingFrame, nsLayoutUtils::RECTS_ACCOUNT_FOR_TRANSFORMS);

Ug. Now I have more questions. What do we mean by bounding frame here? Is the bounding frame the nearest frame completely enclosing this frame... or is it something larger than that? I guess GetAllInFlowRectsUnion is expanding to cover overflow as well... but couldn't we overflow outside the nearest bounding frame? I'm so confused... :(

// Note: the ink overflow rect already combines overflow from the things in-flow, so we don't need to aggregate here like above

This makes me wonder even more whether we should just be always using ink overflow?

I realise you may not have answers to some of this, but it seems like we might need to get some of those answers before we can make an informed decision here. Or I guess we can just try it and see what breaks, but that makes me a little nervous given the craziness on the wild web.

Flags: needinfo?(jteh) → needinfo?(mreschenberg)

What are scrollable and ink overflows? :) I kinda understand the concept of CSS overflow allowing things to flow outside their box, but I imagined that this would change the layout frame size accordingly. Is this not the case; is overflow actually drawn outside the frame? I thought everything that got drawn had to have a frame... but if so, where is the frame for this overflow stuff? Are there separate frames for overflow or something?

Sometimes we have frames that are a fixed size -- imagine, for example, the frame generated by the div in this snippet: data:text/html,<div style="height: 100px; width: 100px; background: green"></div>. The background color fits the frame exactly, creating a 100px green square. If I add text within that div, it won't automatically be truncated to fit within the 100x100 bounds. For example, something like data:text/html,<div style="height: 100px; width: 100px; background: green">HelloWordThisIsAReallyLongSingleWordOfText</div> means the div is still 100px by 100px, but the line within it overflows in the horizontal (inline-size) direction. The text doesn't expand the frame, so the text within the frame gets the green background, but the piece of the text that overflows has no background.

The portion of the text that gets painted outside of the frame is what we call 'ink overflow' or 'visual overflow' (these mean the same thing).

Scrollable overflow is content that doesn't fit within the fixed frame size, but that also overflows the page boundary (or container boundary, if we're in a nested frame); its content that the user would have to scroll to be able to see. So, if I have a browser window that is 100px by 150px and I put 100px by 500px piece of text within the square frame in the previous example, I'd have 50px of ink overflow (the distance between the frame and the page bottom), and then I'd have 350px of scrollable overflow (the content that goes off the page, that I'd need to scroll to see). I'm not clear on whether or not scrollable overflow is a superset of ink overflow, but in any case, there's at least 350px of scrollable overflow in the previous example.

As to your other questions the short answer is: it depends! The long answer is:
We've talked about fixed frames, but frames are dynamically sized; they expand or shrink depending on their contents. You can think of this type of frame kinda like a dynamic text area -- the more you type in it, the larger (or longer) the input box becomes. In this type, there's really no way to "overflow" -- we're always expanding the frame so that its contents are bound within it. If you add something like "overflow: hidden", no ink or scrollable overflow is painted, if you add "overflow:scroll", then no ink overflow is painted, but scrollable overflow is (within the frame, with a scrollbar on the frame itself)

When we talk about overflow, we aren't talking about additional frames -- we're talking about additional rectangles. So for any one frame, there's (a) the rectangle generated by the frame itself, which might be fixed like in my examples (100x100) or dynamic (b) the rectangle generated by the ink overflow and (c) the rectangle generated by the scrollable overflow. These rects (which eventually become display list items if I remember right?) are all "owned" by the frame, but are not necessarily contained within the frame. By definition, overflow occurs outside the frame's bounds.

So I guess the question is, does it make sense to alter how we report position, size info (which I assume is through Bounds() ?) to report something like ink overflow instead of inherent frame size for some subset of accessibles?

I fear my understanding is too limited to know what impact this would have. On the other hand, what we're doing right now is clearly broken somehow, so I think everything is on the table at this point. :)

I guess one question is: if ink/scrollable overflow can report bounds outside a 0-sized frame, I assume they can also report bounds outside of a non-0-sized frame? If so, should we always be taking ink/scrollable overflow into account... somehow?

yeah, this was my feeling -- I think ink overflow is a reasonable choice here. If something is visible but overflows the frame, the visual focus is still on the entire element, not just the frame-contained piece (especially when frames have no visible boundary, like in the icloud example). It might make sense to have AT's "focus" in the same way.

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

*aBoundingFrame = nsLayoutUtils::GetContainingBlockForClientRect(frame);
    nsRect unionRect = nsLayoutUtils::GetAllInFlowRectsUnion(
        frame, *aBoundingFrame, nsLayoutUtils::RECTS_ACCOUNT_FOR_TRANSFORMS);

Ug. Now I have more questions. What do we mean by bounding frame here? Is the bounding frame the nearest frame completely enclosing this frame... or is it something larger than that? I guess GetAllInFlowRectsUnion is expanding to cover overflow as well... but couldn't we overflow outside the nearest bounding frame? I'm so confused... :(

its okay! a bounding frame is something that constrains your ink or scrollable overflow. Think about this example: data:text/html,<div style="height:200px; width:100px; background:green; overflow:hidden;"><div style="background:blue;">HelloIAmSomeReallyLongTextThatOverflowsThisFrame</div></div>

We have an outer frame, of a fixed size, that is "overflow:hidden", this means anything that would be ink or scrollable overflow will be clipped to the frame boundary and won't be painted. The inner frame, however, has no such constraint. It has no fixed size and no overflow rule, which means if it were to stand alone on the page (without its parent), it would expand or shrink to fit the amount of text in it. When I add a really long string of text, like I've done in the example, two things happen:

  • The inner frame expands to the length of the text string (which is longer than 100px). There is no overflow generated by this frame's contents, but this frame itself does generate overflow for the outer frame because its width now exceeds the width of its parent.
  • The outer frame looks at the inner frame's size (greater than its width) and applies the overflow:hidden property, clipping the text off at the outer frame's boundary (so visually only "HelloIAmSome" is painted).

In this example, the out frame is the bounding frame for the inner one. If we didn't have the "overflow:hidden" property on the outer frame, then the bounding frame would be the root frame/web area.

// Note: the ink overflow rect already combines overflow from the things in-flow, so we don't need to aggregate here like above

This makes me wonder even more whether we should just be always using ink overflow?

Yeah this is sorta my thought, and I think this is what Safari and Chrome do -- though I'd love to confirm by dumping the frame trees there. I can reach out to layout folks to see if that's possible :) It'd be good data to have.

I realise you may not have answers to some of this, but it seems like we might need to get some of those answers before we can make an informed decision here. Or I guess we can just try it and see what breaks, but that makes me a little nervous given the craziness on the wild web.

Let me know if you have other questions! hopefully the examples are helpful 🤞

Flags: needinfo?(mreschenberg)
Flags: needinfo?(jteh)

I apologise for my novel lol
TLDR; ink overflow is what gets painted outside of the frame -- its a rectangle not an additional frame. scrollable overflow is content that is painted beyond the scroll area of the frame (ie. beyond the frame's bounding frame). yes, I think maybe we should just be using ink overflow, but I'm sure there'd be things that break because 🤷‍♀️ the web

Thanks so much for your thorough explanation. It was really helpful.

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

Sometimes we have frames that are a fixed size -- imagine, for example, the frame generated by the div in this snippet: data:text/html,<div style="height: 100px; width: 100px; background: green"></div>. The background color fits the frame exactly, creating a 100px green square.

Interestingly, Firefox and Chrome report the same width and height for that example. That suggests Chrome doesn't always use ink overflow.

If I add text within that div, it won't automatically be truncated to fit within the 100x100 bounds. For example, something like data:text/html,<div style="height: 100px; width: 100px; background: green">HelloWordThisIsAReallyLongSingleWordOfText</div> means the div is still 100px by 100px, but the line within it overflows in the horizontal (inline-size) direction.

In this example, I notice that the text leaf (backed by a text frame) reports a much larger width, which I assume includes the overflow. I guess that makes sense, but now I'm starting to see cases where using ink overflow could get nasty. Authors might deliberately have some kind of tiny container with a huge ink overflow and we probably wouldn't want the bounds of the container to cover the world. :)

Some testing in Chrome revealed that the width/height only cover the overflow if the width or height is set to 0px. So this:
data:text/html,<div style="height: 1px; width: 1px; background: green">HelloWordThisIsAReallyLongSingleWordOfText</div>
resulted in a width/height of 3/3. This:
data:text/html,<div style="height: 0px; width: 0px; background: green">HelloWordThisIsAReallyLongSingleWordOfText</div>
resulted in a width/height of 798/45. That would suggest that they're only using ink overflow for a width or height of 0, which is exactly what your "temp fix" is doing. :)

When we talk about overflow, we aren't talking about additional frames -- we're talking about additional rectangles.

The text frame does seem to cover the overflow, though. So is it correct to say that the overflow is encompassed within some frame, but not the frame being overflowed?

*aBoundingFrame = nsLayoutUtils::GetContainingBlockForClientRect(frame);
    nsRect unionRect = nsLayoutUtils::GetAllInFlowRectsUnion(
        frame, *aBoundingFrame, nsLayoutUtils::RECTS_ACCOUNT_FOR_TRANSFORMS);

a bounding frame is something that constrains your ink or scrollable overflow.

So this code is always constraining our rect, not expanding it? That is, unionRect will always be smaller than frame's bounds, not larger?

Anyway, it seems to me that your so-called temp fix is actually the right (or at least best possible) fix here and it seems to be what Chrome is doing too.

Flags: needinfo?(jteh)
See Also: → 1694716

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

When we talk about overflow, we aren't talking about additional frames -- we're talking about additional rectangles.

The text frame does seem to cover the overflow, though. So is it correct to say that the overflow is encompassed within some frame, but not the frame being overflowed?

Yes, sorry I meant to say that overflow doesn't create any additional frames. It is still technically encompassed by some bounding frame. We can't, for example, overflow the web area :)

*aBoundingFrame = nsLayoutUtils::GetContainingBlockForClientRect(frame);
    nsRect unionRect = nsLayoutUtils::GetAllInFlowRectsUnion(
        frame, *aBoundingFrame, nsLayoutUtils::RECTS_ACCOUNT_FOR_TRANSFORMS);

a bounding frame is something that constrains your ink or scrollable overflow.

So this code is always constraining our rect, not expanding it? That is, unionRect will always be smaller than frame's bounds, not larger?

Yes, that's my understanding. Also, the returned rect will be in the coord space of our bounding frame.

Firing off a new bug to land work that correctly sets Bounds() so the SR cursor tracks the button
Sending the action is still an issue. Jamie and I discovered last week that getting the button node in the inspector and sending it a .click() works, which is confusing because our mouse event should target the node as well and do (essentially?) the same thing.
Right now we're betting that this isn't a coordinate issue since I couldn't drill down where the x, y coordinates of our event are actually used, and Jamie mentioned we typically send the event to the node directly instead of simulating a screen-coord based event (but we do do that for hit testing).

Depends on: 1695716

Comment on attachment 9205297 [details]
Bug 1649239: Use ink overflow rect to calculated relative bounds when frame area is zero r?jamie,emilio

Revision D106384 was moved to bug 1695716. Setting attachment 9205297 [details] to obsolete.

Attachment #9205297 - Attachment is obsolete: true

I've just spent hours trying to figure this out and haven't managed to solve it cleanly. :( Here's some additional info though.

  1. The coordinates from our event do get provided via JS; e.g. event.client/screenX/Y. It's possible that iCloud uses these somehow.
  2. element.click() generates a "positionless" event; i.e. client/screenX/Y are 0. Perhaps iCloud has an exception for this case?
  3. You can't seem to dispatch a positionless event (setting event.mFlags.mIsPositionless = true) using PresShell::HandleEventWithTarget. nsGenericHTMLElement::Click uses EventDispatcher::Dispatch instead.
  4. The coordinates we get from nsIFrame::GetNearestWidget seem to be wrong. The size is 0 x 0, which is expected here.
  5. Using InkOverFlowRectRelativeToSelf and transforming to the root frame is closer, though I think I'm still missing something there related to views/widgets.
  6. Getting rid of the touchstart/end and mousedown/up events and replacing them with a click doesn't work.
  7. I attached a JS click event listener to the button and logged the screenX and screenY coordinates. Even hard-coding adjusted coordinates in LocalAccessible::DispatchClickEvent so that the logged coords are the same between that and a real, working click doesn't work! This probably suggests the problem isn't coordinates.
  8. Calling nsGenericHTMLElement::Click in LocalAccessible::DispatchClickEvent does work. However, that won't do mousedown and mouseup events and I suspect it won't be treated as real user interaction by Firefox. So, even if we only do this for 0 x 0 frames, this probably isn't an acceptable solution.
  9. Chromium's equivalent of Accessible::DoAction doesn't dispatch positionless events, but it does work. From what I can tell, it too dispatches mousedown and mouseup events. I really don't know why Chromium's works and ours doesn't.

I've re-tested this and found that the issue as originally reported seems fixed. I'm able to press Enter to access the Mail, Calendar, Notes, and Photos pages, among all the others. What were apparently formerly buttons are now links, all of which appear to work just fine with NVDA on Windows. If we have some reproducer of the click event issues, I'm happy to take a look at those, but as it stands it seems like Apple might have done something on their end (read: changed the website) to make this work.

Jamie and Morgan - do either of you have markup that can reproduce the click problem? If I try to modify Morgan's example test case such that the button follows a link, like so:

data:text/html,<button onClick="parent.location='https://google.com'" style="text-align: center; transform: matrix(1, 0, 0, 1, 292.5, 164.583); inline-size:0; block-size:0;">hello world</div>

the button works as expected with NVDA. Maybe this issue can be closed?

Flags: needinfo?(mreschenberg)
Flags: needinfo?(jteh)

That example also works for me with VO

I think somewhere along the way I got an apple eng to file a bug on this for me -- maybe they got around to fixing it?
Or maybe our CtW bounds code is doing us some favours :)
In either case, I don't have other markup to test and it seems like this is best resolved as WORKSFORME

Flags: needinfo?(mreschenberg)

Great, thanks :)

Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(jteh)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: