New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 651145 link

Starred by 1 user
Status: Verified
Owner:
dmazz...@chromium.org
Closed: Oct 2016
Cc:
█ brajkumar@chromium.org
mac-bugs-priority@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

GetAttributedTextForTextMarkerRange is crashing

Project Member Reported by dmazz...@chromium.org, Sep 28 2016 
Repro for me:
1. Either run in debug mode or enable DCHECK assertions
2. Turn on VoiceOver
3. Open Google Groups
4. Click on an existing thread
5. Click on a thread or press 'a' to reply
6. Start typing misspelled words

I'm hitting the DCHECK assertions in browser_accessibility_cocoa.mm, in particular:

DCHECK_GE(range_length, 0);

I added some logging before that and found that the start object and end object are not the same, and I get:

[29034:775:0928/104323:ERROR:browser_accessibility_cocoa.mm(288)] Start offset: 4
[29034:775:0928/104323:ERROR:browser_accessibility_cocoa.mm(290)] End offset: 4
[29034:775:0928/104323:ERROR:browser_accessibility_cocoa.mm(297)] Text length: 3
[29034:775:0928/104323:ERROR:browser_accessibility_cocoa.mm(298)] Trim length: 0
[29034:775:0928/104323:ERROR:browser_accessibility_cocoa.mm(301)] range_length: -1
[29034:775:0928/104323:FATAL:browser_accessibility_cocoa.mm(302)] Check failed: range_length >= 0 (-1 vs. 0)

If DCHECKs are not on, the call to NSMakeRange returns nil and this leads to a crash a few lines later.

This is a top crasher for one user (>10x per day)

Comment 1 by dmazz...@chromium.org, Sep 28 2016

Components: UI>Accessibility

Comment 2 by dmazz...@chromium.org, Sep 28 2016

Labels: M-54 ReleaseBlock-Stable
This is a small, safe workaround for the crash for merging:

https://codereview.chromium.org/2374103002/

We can follow that up with a proper fix.

Comment 3 by ligim...@chromium.org, Sep 29 2016 

Thanks for the fix.

FYI: M54 will be promoted to stable soon, so please try to have this bug fixed no later than first week of October.Also ensure that change is baked/verified in Canary before requesting a merge.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 1 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/681be131bedcf117065ef1d945395255ad534bb9

commit 681be131bedcf117065ef1d945395255ad534bb9
Author: dmazzoni <dmazzoni@chromium.org>
Date: Sat Oct 01 02:10:49 2016

Temporarily work around crash when adding misspelling annotations to accessible strings

BUG= 651145 

Review-Url: https://codereview.chromium.org/2374103002
Cr-Commit-Position: refs/heads/master@{#422282}

[modify] https://crrev.com/681be131bedcf117065ef1d945395255ad534bb9/content/browser/accessibility/browser_accessibility_cocoa.mm

Comment 5 by ligim...@chromium.org, Oct 3 2016 

Dominic, could you please verify the issue and if all looks good please request a merge to M54 ASAP

Comment 6 by dmazz...@chromium.org, Oct 4 2016

Labels: Merge-Request-54
Owner: dmazz...@chromium.org
Status: Fixed (was: Assigned)
Manually verified that the fix is working, merge requested.

Comment 7 by dimu@chromium.org, Oct 4 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 4 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d23d3642232eec5c863130d15946a9e3cd55ec4e

commit d23d3642232eec5c863130d15946a9e3cd55ec4e
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Tue Oct 04 19:52:11 2016

Merge to M54: Temporarily work around crash when adding misspelling annotations to accessible strings

BUG= 651145 

Review-Url: https://codereview.chromium.org/2374103002
Cr-Commit-Position: refs/heads/master@{#422282}
(cherry picked from commit 681be131bedcf117065ef1d945395255ad534bb9)

Review URL: https://codereview.chromium.org/2396593002 .

Cr-Commit-Position: refs/branch-heads/2840@{#639}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/d23d3642232eec5c863130d15946a9e3cd55ec4e/content/browser/accessibility/browser_accessibility_cocoa.mm

Comment 9 by brajkumar@chromium.org, Oct 5 2016

Cc: brajkumar@chromium.org
Labels: Needs-Feedback
dmazzoni@ - Is there any manual repro steps available to verify this issue from Chrome-TE end?

Comment 10 by ligim...@chromium.org, Oct 5 2016

Status: Verified (was: Fixed)
Dominic verified it as per #6 , so tagging as verified.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 27 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d23d3642232eec5c863130d15946a9e3cd55ec4e

commit d23d3642232eec5c863130d15946a9e3cd55ec4e
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Tue Oct 04 19:52:11 2016

Merge to M54: Temporarily work around crash when adding misspelling annotations to accessible strings

BUG= 651145 

Review-Url: https://codereview.chromium.org/2374103002
Cr-Commit-Position: refs/heads/master@{#422282}
(cherry picked from commit 681be131bedcf117065ef1d945395255ad534bb9)

Review URL: https://codereview.chromium.org/2396593002 .

Cr-Commit-Position: refs/branch-heads/2840@{#639}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/d23d3642232eec5c863130d15946a9e3cd55ec4e/content/browser/accessibility/browser_accessibility_cocoa.mm
►

Sign in to add a comment