Bug 95618 – [Stable] [GTK] Crash in WebCore::HTMLSelectElement::selectedIndex

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

Just saw this crash with the stable branch (1.9.90). Marking critical since this is a crash in the stable series. I caused this to happen by clicking on my inbox in Yahoo mail.


#0  0x00007ffff629e8f8 in WebCore::HTMLSelectElement::selectedIndex() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#1  0x00007ffff629ea25 in WebCore::HTMLSelectElement::activeSelectionStartListIndex() const ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#2  0x00007ffff6e5293d in WebCore::AXObjectCache::postPlatformNotification(WebCore::AccessibilityObject*, WebCore::AXObjectCache::AXNotification) () from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#3  0x00007ffff5f1e47c in WebCore::AXObjectCache::notificationPostTimerFired(WebCore::Timer<WebCore::AXObjectCache>*)
    () from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#4  0x00007ffff65e4742 in WebCore::ThreadTimers::sharedTimerFiredInternal() ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#5  0x00007ffff6e8b842 in WebCore::timeout_cb(void*) ()
   from /home/martin/work/gnome-shell/install/lib/libwebkitgtk-3.0.so.0
#6  0x00007ffff3cbefab in g_timeout_dispatch (source=<optimized out>, callback=<optimized out>, 
    user_data=<optimized out>) at gmain.c:4026
#7  0x00007ffff3cbe3e3 in g_main_dispatch (context=0x6f6d50) at gmain.c:2715
#8  g_main_context_dispatch (context=0x6f6d50) at gmain.c:3219
#9  0x00007ffff3cbe730 in g_main_context_iterate (dispatch=1, block=<optimized out>, context=0x6f6d50, 
    self=<optimized out>) at gmain.c:3290
#10 g_main_context_iterate (context=0x6f6d50, block=<optimized out>, dispatch=1, self=<optimized out>) at gmain.c:3227
#11 0x00007ffff3cbe7f4 in g_main_context_iteration (context=0x6f6d50, may_block=1) at gmain.c:3351
#12 0x00007ffff42888b4 in g_application_run (application=0x7fa070, argc=<optimized out>, argv=0x7fffffffd888)
    at gapplication.c:1607
#13 0x000000000042dd85 in main (argc=1, argv=0x7fffffffd888) at ephy-main.c:493

Adding Mario to the CC since the stack intersects with the accessibility code.

[Mario Sanchez Prada]

Created attachment 161899 [details]
Patch proposal

Simple patch to fix this issue, by protecting us against the situation exposed here: a selector which is not rendered through a HTML <select> element.

I propose this patch because the current code was assuming that was the case (a HTML <select> element was always there), which seems to be plainly wrong, as this situation clearly exposes. Of course, some better improvements could be done (e.g. supporting WAI-ARIA selectors), but I think the focus should be now put on fixing this crash, and so that's what this patch does, by ensuring we only emit the signal when such an assumption is right.

[Martin Robinson]

Comment on attachment 161899 [details]
Patch proposal

Great work!! Is it easy to reproduce this crash? Perhaps we could add a simple test case?

[Mario Sanchez Prada]

Created attachment 161992 [details]
Patch proposal + Layout test

Good idea. I'm attaching a new patch with a simple test case that crashes without the patch (and obviously doesn't with it)

[Martin Robinson]

Comment on attachment 161992 [details]
Patch proposal + Layout test

View in context: https://bugs.webkit.org/attachment.cgi?id=161992&action=review

Great! Do you mind adding this to the list at: https://trac.webkit.org/wiki/WebKitGTK/1.10.x ?

> LayoutTests/platform/gtk/accessibility/aria-listbox-crash.html:34
> +   window.setTimeout("testRunner.notifyDone()",0);

Just a little nit here: you're missing a space after the comma, I think.

[WebKit Review Bot]

Comment on attachment 161992 [details]
Patch proposal + Layout test

Clearing flags on attachment: 161992

Committed r127466: <http://trac.webkit.org/changeset/127466>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Mario Sanchez Prada]

(In reply to comment #4)
> (From update of attachment 161992 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161992&action=review
> 
> Great! Do you mind adding this to the list at: https://trac.webkit.org/wiki/WebKitGTK/1.10.x ?

Already did yesterday.

> > LayoutTests/platform/gtk/accessibility/aria-listbox-crash.html:34
> > +   window.setTimeout("testRunner.notifyDone()",0);
> 
> Just a little nit here: you're missing a space after the comma, I think.

Sure, sorry about it. Anyway, now the patch has been already committed so I think it makes no sense to commit a "fix" just for that "issue" :)

Thanks