Closed Bug 1523905 Opened 7 years ago Closed 7 years ago

Crash when browsing Gmail with accessibility enabled on linux

Categories

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

Product:
Core
Component:
Disability Access APIs
Version:
66 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- wontfix
firefox66 --- fixed
firefox67 --- verified

People

(Reporter: pvagner, Assigned: MarcoZ)

References

Details

(Keywords: crash, regression)

Crash Data

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

  • Make sure Orca is running
  • Start firefox, browse and login to gmail.com
  • Expand a conversation
  • Make sure orca is switched into browse mode by pressing insert+a
  • Press ctrl+home to move to the top
  • Press orca quick navigation keyboard shortcut h until orca presents sender of a first message in the conversation
  • Now either press down arrow multiple times to browse over the toolbar buttons and translation options or press up arrow key multiple times to navigate out of the conversation.

Actual results:

Firefox crashes like this.... https://crash-stats.mozilla.org/signature/?product=Firefox&signature=mozilla%3A%3Aa11y%3A%3ATableAccessible%3A%3ARowIndexAt&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports

Guessing from the reports it's happening for stable, beta and nightly channels.
I am using Firefox nightly and I need to arrow up in the last step of my steps to trigger the crash. Other users can reproduce this when pressing down arrow: https://mail.gnome.org/archives/orca-list/2019-January/msg00376.html

Expected results:

Firefox should not crash and it should be possible to continue browsing.

Adding more info to this bug. I see crashes in 66 nightly and 64, but none showing up in 65 or 67 yet.

Status: UNCONFIRMED → NEW
Crash Signature: [@ mozilla::a11y::TableAccessible::RowIndexAt]
status-firefox66: --- → affected
Component: Untriaged → Disability Access APIs
Ever confirmed: true
Keywords: crash, regression, regressionwindow-wanted
Product: Firefox → Core

Peter, can you please confirm whether the following test case causes the crash?

data:text/html,<style>table, tbody, tr, td { display: block; }</style>before<table><tbody><tr><td>crash</td></tr></tbody></table>after

This is a distilled version of what occurs around the "Reply" and "More" buttons below the sender heading.

This is all kinds of messed up:

  1. The table reports row and column counts of 0.
  2. That in turn causes the table-cell-index attribute to report -1, etc. for the cells. This should never be negative.
  3. I'm not quite sure what happens from here because the ATK getRowAtIndexCB implementation explicitly checks for negative indexes and returns early with -1 if this happens. Maybe Orca is flipping the negative cell index? (Again, we shouldn't be giving it a negative index in the first place.) Our ATK implementation doesn't bounds check aside from < 0, so I guess passing a positive value would fail here. (In contrast, our ia2AccessibleTable::get_rowIndex does check the upper bound.)
  4. TableAccessible::RowIndexAt takes the cell index and just divides by the ColCount. Since ColCount is 0, we divide by 0 and crash.

As for a fix:

  1. Fixing bug 1461244 should fix this, since the table won't get a 0 ColCount.
  2. We really should protect against bugs like this, though. We should:
    • Add better sanity checking in ARIAGridCellAccessible::NativeAttributes so that we never expose negative indexes for table-cell-index; and
    • bounds check in the ATK getRowAtIndexCB implementation like we do in ia2AccessibleTable. Or perhaps we should move that check into the TableAccessible::RowIndexAt implementation.
Flags: needinfo?(pvagner)
Priority: -- → P2

Jamie huge thanks for trying to make this work and for comprehensive comment explaining the whole situation.
Unfortunatelly I am unable to reproduce the crash with your test case.
I have even tried tweaking it a little by adding a div or a button or more rows into the table. Still I can't trigger the crash with the test case no matter what I'm doing.
As I am further playing with this, I have discovered that it is not crashing all the time on the gmail site either. My rough guess is that it's about 8 times from 10 I can make it crash on gmail. When it does not crash immediatelly moving up and down several times makes it crash.
Might it be related to the fact orca is controlling navigation trying to set focus on focusable controls inside the table?

Flags: needinfo?(pvagner)

Peter, can you still reproduce the crash with this try build, which contains a proposed fix for bug 1461244:
https://queue.taskcluster.net/v1/task/Tt67jUi1SHya4O9Sgp7fSA/runs/0/artifacts/public/build/target.tar.bz2

Flags: needinfo?(pvagner)

I can no longer reproduce the crash with this try build.
I tried a few times with different conversations opened in gmal.

Flags: needinfo?(pvagner)

Thank you, Peter! Marking this bug as depending on the other one. It can be closed once that bug is fixed.

Depends on: 1461244
OS: Unspecified → All
Hardware: Unspecified → All

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

  1. We really should protect against bugs like this, though. We should:
    • Add better sanity checking in ARIAGridCellAccessible::NativeAttributes so that we never expose negative indexes for table-cell-index; and
    • bounds check in the ATK getRowAtIndexCB implementation like we do in ia2AccessibleTable. Or perhaps we should move that check into the TableAccessible::RowIndexAt implementation.

Filed Bug 1524919 for this and am working on it.

Fixed by bug 1461244, which should be in the upcoming 2019-02-04 Nightly build.

Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Assignee: nobody → mzehe
status-firefox65: --- → wontfix
status-firefox-esr60: --- → unaffected

I'd love to have this fixed in 66 as well, will follow up in bug 1461244.

Removing the regressionwindow-wanted keyword since the issue is already RESOLVED FIXED.

Keywords: regressionwindow-wanted
Flags: qe-verify+

I’ve managed to reproduce the crash with Fx 67.0a1 (2019-01-30) on Ubuntu 18.04 x64.
Issue is no longer reproducible, using the same platform with Fx 68.0a1 (2019-05-09) and Fx 67.0b18.

Status: RESOLVED → VERIFIED
status-firefox67: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.