Closed Bug 1203861 Opened 4 years ago Closed 4 years ago

Infinite accessible name change events when hovering mouse over certain MIME-encoded subjects

Categories

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

Unspecified
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox45 --- fixed
firefox46 --- fixed

People

(Reporter: jdiggs, Assigned: tbsaunde)

References

(Blocks 1 open bug)

Details

(Keywords: hang, testcase)

Attachments

(4 files)

Steps to reproduce:
1. Import the attached mail message in Thunderbird
2. Quit Thunderbird
3. Launched the attached pyatspi accessible event listener in a terminal
4. Launch Thunderbird
5. Click on the mail folder which holds the imported mail message
6. Hover the mouse pointer over the imported mail message in the message list
   which should cause a tooltip with the subject to popup

Results:
1. The listener will (after a slight delay) spew out 20,000 object:property-change:accessible-name events for the tooltip and then bail. An Orca user provided me with debut output with over 500,000 events.

2. Thunderbird becomes (and remains, from what I've seen) non-responsive and needs to be terminated. (Stacktrace will follow.)

Note: If you perform the steps with other messages, things work as expected. As best as I can tell, the bug only happens with certain MIME-encoded subjects.
Blocks: orca
Keywords: hang, testcase
Ping? I just had to force quit Thunderbird because of this bug.
Severity: major → critical
AtkObject stores the name as a C string, but nsCString can store strings with
'\0' in the middle.  That means that if we compute a name containing '\0'
nsCString.equals() will nevr consider that string to be equal to
AtkObject::name, however we only want to compare up to the first '\0' because
that is the only part Atk will ever see.  So we should use strncmp() instead of
nsCString.equals().
Attachment #8704836 - Flags: review?(dbolter)
Comment on attachment 8704836 [details] [diff] [review]
use strncmp() instead of nsCString.equals() to compare names

Review of attachment 8704836 [details] [diff] [review]:
-----------------------------------------------------------------

I love your commit messages.
Attachment #8704836 - Flags: review?(dbolter) → review+
https://hg.mozilla.org/mozilla-central/rev/9133117f410c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 46.0
Assignee: nobody → tbsaunde+mozbugs
Comment on attachment 8704836 [details] [diff] [review]
use strncmp() instead of nsCString.equals() to compare names

Approval Request Comment
[Feature/regressing bug #]:none
[User impact if declined]:crashes with email subjects containing null bytes, and probably other places where the names of things contain '\0'
[Describe test coverage new/current, TreeHerder]: on m-c so for a week without regression, and manually tested the bug in thunderbird is gone.
[Risks and why]:  low, it should mean in the worst case we don't fire an event we should.
[String/UUID change made/needed]:
Attachment #8704836 - Flags: approval-mozilla-aurora?
Trevor, does this has any potential impact on Firefox?
Thanks
Flags: needinfo?(tbsaunde+mozbugs)
(In reply to Sylvestre Ledru [:sylvestre] from comment #9)
> Trevor, does this has any potential impact on Firefox?
> Thanks

yes, I suspect that you can trigger the same bug in firefox rather easily, but I have not tried.  Its a rather odd edge case so I'm not totally suprised we haven't heard about it being a problem in Firefox.
Flags: needinfo?(tbsaunde+mozbugs)
Comment on attachment 8704836 [details] [diff] [review]
use strncmp() instead of nsCString.equals() to compare names

OK, let's take it. We still can always backout this patch if it causes unexpected regressions in Firefox.
Attachment #8704836 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Moving to Core so I can set the flags to stop showing this in our uplift queries.
Component: Disability Access → Disability Access APIs
Product: Thunderbird → Core
Target Milestone: Thunderbird 46.0 → mozilla46
You need to log in before you can comment on or make changes to this bug.