Closed Bug 630841 Opened 9 years ago Closed 9 years ago

Crash in nsHyperTextAccessible::GetText()

Categories

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

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla2.0b11
Tracking Status
blocking2.0 --- final+

People

(Reporter: msucan, Assigned: fherrera)

Details

(Keywords: access, regression, Whiteboard: [softblocker])

Attachments

(2 files, 1 obsolete file)

Firefox 4 very easily crashes when I use Panorama (just drag/resize groups).

System info:

Pentium(R) Dual-Core CPU E6500 @ 2.93GHz, Ubuntu 10.04 LTS (amd64), Gnome 2.30.2, Metacity, Nvidia drivers (195.36.24), Xorg 1.7.6 ... all coming from the official Ubuntu repos (up-to-date).

Gnome has accessibility features enabled. Environment variable: GTK_MODULES="canberra-gtk-module:gail:atk-bridge".

Firefox 4 nightlies do not crash, only Firefox 4 local builds do. The .mozconfig I have is:

. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/ff-obj
mk_add_options MOZ_MAKE_FLAGS="-j4 -s"
ac_add_options --enable-optimize
ac_add_options --enable-tests

If I disable the Gnome accessibility features, Firefox no longer crashes.

Stack trace:

#0  0x00007ffff6bff607 in nsHyperTextAccessible::GetText(int, int, nsAString_internal&) () from ./libxul.so
#1  0x00007ffff6c053f8 in getTextCB () from ./libxul.so
#2  0x00007fffea790cd4 in ?? () from /usr/lib/gtk-2.0/modules/libatk-bridge.so
#3  0x00007ffff30b3fb8 in ?? () from /usr/lib/libgobject-2.0.so.0
#4  0x00007ffff30b5a76 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0
#5  0x00007ffff30b5dc8 in g_signal_emit_by_name () from /usr/lib/libgobject-2.0.so.0
#6  0x00007ffff6c00241 in nsAccessibleWrap::FireAtkTextChangedEvent(AccEvent*, _AtkObject*) () from ./libxul.so
#7  0x00007ffff6c00f3c in nsAccessibleWrap::FirePlatformEvent(AccEvent*) () from ./libxul.so
#8  0x00007ffff6bea964 in nsEventShell::FireEvent(AccEvent*) () from ./libxul.so
#9  0x00007ffff6bd9b57 in nsDocAccessible::ProcessPendingEvent(AccEvent*) () from ./libxul.so
#10 0x00007ffff6bd0e78 in NotificationController::WillRefresh(mozilla::TimeStamp) () from ./libxul.so
#11 0x00007ffff6403034 in nsRefreshDriver::Notify(nsITimer*) () from ./libxul.so
#12 0x00007ffff6d4f656 in nsTimerImpl::Fire() () from ./libxul.so
#13 0x00007ffff6d4f711 in nsTimerEvent::Run() () from ./libxul.so
#14 0x00007ffff6d4c918 in nsThread::ProcessNextEvent(int, int*) () from ./libxul.so
#15 0x00007ffff6d18f81 in NS_ProcessNextEvent_P(nsIThread*, int) () from ./libxul.so
#16 0x00007ffff6c64b8a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) () from ./libxul.so
#17 0x00007ffff6d7dc56 in MessageLoop::Run() () from ./libxul.so
#18 0x00007ffff6baf6b7 in nsBaseAppShell::Run() () from ./libxul.so
#19 0x00007ffff6a668ce in nsAppStartup::Run() () from ./libxul.so
#20 0x00007ffff628e995 in XRE_main () from ./libxul.so
#21 0x0000000000401c59 in main ()
regression from bug 630001, part3 (fix getText).
blocking2.0: --- → ?
Keywords: access, regression
508     nsAccessible* child = GetChildAt(startChildIdx);
509     child->AppendTextTo(aText, aStartOffset - childOffset,
510                         aEndOffset - aStartOffset);

child is NULL because GetChildAt returns an wrong result (that child has been removed, but mOffsets has not been updated).
Attachment #509092 - Flags: review?(surkov.alexander)
Attachment #509092 - Flags: review?(surkov.alexander) → review+
blocking2.0: ? → final+
Whiteboard: [softblocker]
Assignee: nobody → fherrera
landed on 2.0 (fx4beta12) - http://hg.mozilla.org/mozilla-central/rev/6cb543f7d5b0
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
fer, could you create mochitest for this please?
Flags: in-testsuite+
Flags: in-testsuite+ → in-testsuite?
Attached patch mochitest case (obsolete) — Splinter Review
Attachment #509148 - Flags: review?(surkov.alexander)
Comment on attachment 509148 [details] [diff] [review]
mochitest case


>+    function removeChild(aContainerID, aChildID, aInitialText, aFinalText)

please add a comment before the function what it's going to test like

/**
 * Text offsets must be updated when hypertext child is removed.
 */

>+    {
>+      this.containerNode = getNode(aContainerID);
>+      this.container = getAccessible(this.containerNode, nsIAccessibleText);
>+      this.childNode = getNode(aChildID);
>+      this.child = getAccessible(this.childNode, nsIAccessibleHyperText);

this.child isn't used.

>+      this.initialText = aInitialText;
>+      this.finalText = aFinalText;

well you don't need to store them as members, you could refer as aInitialText and aFinalText

>+
>+      // Call first to getText so text is cached

the text doesn't get cached by this call, but offsets.

>+      this.finalCheck = function removeChild_finalCheck()
>+      {
>+        is(this.container.getText(0, -1), this.finalText,
>+           "Wrong text after child removal");
>+        is(this.container.characterCount, this.finalText.length,
>+           "Wrong text after child removal");
>+        for(var i = 0; i < this.finalText.length; i++) {
>+          is(this.container.getText(i, i + 1), this.finalText[i],
>+             "Wrong text after child removal");
>+        }

this loop isn't necessary actually, up to you to save it or get rid

>+      }
>+
>+      this.getID = function updateText_getID()

removeChild_getID()

>+      {
>+        return "check text after removing chuld from '" + aContainerID + "'";

chuld -> child

looks good, thanks, r=me, please
Attachment #509148 - Flags: review?(surkov.alexander) → review+
Attachment #509148 - Attachment is obsolete: true
FYI, I got this crash with 4.0b11 build1 (which is not unexpected given when the patch landed).
OS: Linux → All
OS: All → Linux
(In reply to comment #7)
> Created attachment 509152 [details] [diff] [review]
> updated test case

I put it into my landing queue. I'll land eventually if you don't need it asap. Thanks for doing this!
Flags: in-testsuite? → in-testsuite+
Target Milestone: mozilla2.0b12 → mozilla2.0b11
Cannot reproduce it anymore with 4.0b11 build3
Setting to Verified basec on comment #12.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.