Bug 72624 – Seeing crash in DRT after loading-iframe-sends-notification.html land

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

2011-11-17 08:58:39.760 DumpRenderTree[27057:903] -[WebDynamicScrollBarsView accessibilitySetShouldRepostNotifications:]: unrecognized selector sent to instance 0x113b410b0
2011-11-17 08:58:39.787 DumpRenderTree[27057:903] An uncaught exception was raised
2011-11-17 08:58:39.796 DumpRenderTree[27057:903] -[WebDynamicScrollBarsView accessibilitySetShouldRepostNotifications:]: unrecognized selector sent to instance 0x113b410b0
2011-11-17 08:58:39.866 DumpRenderTree[27057:903] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[WebDynamicScrollBarsView accessibilitySetShouldRepostNotifications:]: unrecognized selector sent to instance 0x113b410b0'
*** Call stack at first throw:
(
	0   CoreFoundation                      0x00007fff816517b4 __exceptionPreprocess + 180
	1   libobjc.A.dylib                     0x00007fff83b58f03 objc_exception_throw + 45
	2   CoreFoundation                      0x00007fff816ab110 +[NSObject(NSObject) doesNotRecognizeSelector:] + 0
	3   CoreFoundation                      0x00007fff8162391f ___forwarding___ + 751
	4   CoreFoundation                      0x00007fff8161fa68 _CF_forwarding_prep_0 + 232
	5   DumpRenderTree                      0x00000001000053e8 -[AccessibilityNotificationHandler initWithPlatformElement:] + 87
	6   DumpRenderTree                      0x00000001000087f0 _ZN22AccessibilityUIElement23addNotificationListenerEP13OpaqueJSValue + 72
	7   DumpRenderTree                      0x00000001000037e6 _ZL31addNotificationListenerCallbackPK15OpaqueJSContextP13OpaqueJSValueS3_mPKPKS2_PS5_ + 68
	8   JavaScriptCore                      0x000000010023aad8 _ZN3JSC18JSCallbackFunction4callEPNS_9ExecStateE + 392
)
terminate called after throwing an instance of 'NSException'

2011-11-17 08:58:42,756 26818 worker.py:158 DEBUG worker/0 killing driver
2011-11-17 08:58:42,757 26668 printing.py:462 INFO   accessibility/loading-iframe-sends-notification.html -> unexpected DumpRenderTree crash


saw it at
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Tests%29/builds/34783/steps/layout-test/logs/stdio

[Dominic Mazzoni]

Looking...

[chris fleizach]

Also looks like the test might fail in WK2

http://build.webkit.org/results/Lion%20Intel%20Debug%20(WebKit2%20Tests)/r100636%20(1840)/results.html

[Dominic Mazzoni]

The problem is that the accessibility object for the iframe element gets a role of UnknownRole, and the Mac implementation of accessibilityPlatformIncludesObject skips nodes with an unknown role. So it's impossible for the layout test to get an AccessibilityUIElement corresponding to the iframe element on Mac, it can only get the parent or child of it.

1. I think the best solution is to make an iframe element part of the accessibility tree by adding a new role IFrameRole, which could just map to GroupRole on Mac - that way it wouldn't be ignored. I don't see any harm, and there might be some benefit, because a web developer could add important attributes to the iframe element that we wouldn't want to ignore.

2. An alternative solution would be for the notification when an iframe loads to be posted on the first unignored parent. That way it'd be posted on the parent of the iframe on Mac and the test could be adapted to handle either case.

3. Or, I could just move this test to platform/chromium, since this feature isn't needed on Mac.

[Dominic Mazzoni]

I added this test to platform/mac/Skipped in the meantime. Chris, does my solution #1 (create a role for iframe element so it's not ignored on Mac) sound okay?

[chris fleizach]

(In reply to comment #3)
> The problem is that the accessibility object for the iframe element gets a role of UnknownRole, and the Mac implementation of accessibilityPlatformIncludesObject skips nodes with an unknown role. So it's impossible for the layout test to get an AccessibilityUIElement corresponding to the iframe element on Mac, it can only get the parent or child of it.
> 

That's because that object is an isAttachment() == true. It's attachment view is the one that returns the role.

There should be a web area for the iframe that you can get at. or a scroll area. there are other tests that use iframes that you might look at to see what they do. 

you might also check if the platform element responds to that type of selector before calling it

> 1. I think the best solution is to make an iframe element part of the accessibility tree by adding a new role IFrameRole, which could just map to GroupRole on Mac - that way it wouldn't be ignored. I don't see any harm, and there might be some benefit, because a web developer could add important attributes to the iframe element that we wouldn't want to ignore.
> 

that would break the attachment part.

> 2. An alternative solution would be for the notification when an iframe loads to be posted on the first unignored parent. That way it'd be posted on the parent of the iframe on Mac and the test could be adapted to handle either case.
> 
> 3. Or, I could just move this test to platform/chromium, since this feature isn't needed on Mac.

[Dominic Mazzoni]

> There should be a web area for the iframe that you can get at. or a scroll area. there are other tests that use iframes that you might look at to see what they do.

The problem is that when an iframe reloads, the renderer for the scroll area and web area get deleted and recreated - so for a test, you can't add a notification listener to the iframe's web area, because when the iframe loads, the notification gets sent to the new web area, not the one you were listening to.

Should we have a global addNotificationListener that listens to notifications on any element?

Either that, or I can just do option #2 and send the notification on the first unignored parent.

[chris fleizach]

(In reply to comment #6)
> > There should be a web area for the iframe that you can get at. or a scroll area. there are other tests that use iframes that you might look at to see what they do.
> 
> The problem is that when an iframe reloads, the renderer for the scroll area and web area get deleted and recreated - so for a test, you can't add a notification listener to the iframe's web area, because when the iframe loads, the notification gets sent to the new web area, not the one you were listening to.
> 
> Should we have a global addNotificationListener that listens to notifications on any element?
> 

I think we should do this. I think part of this might be in place. if you look at accessibilityController there's some stuff to do notifications there.

> Either that, or I can just do option #2 and send the notification on the first unignored parent.

[Dominic Mazzoni]

> > Should we have a global addNotificationListener that listens to notifications on any element?

> I think we should do this. I think part of this might be in place.
> if you look at accessibilityController there's some stuff to do notifications there.

Now that the global addNotificationListener has landed, I've simplified this patch and rewritten the test to use it. Now the test is cross-platform with identical expectations on all platforms that support the needed test methods. Tested on Mac (DRT and WKTR) and Chromium. Will upload shortly.

[Dominic Mazzoni]

Created attachment 119318 [details]
Patch

[chris fleizach]

Comment on attachment 119318 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:8
> +

bug title should be first. the description of what's happening should be after the reviewed by

[Dominic Mazzoni]

Created attachment 119326 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 119326 [details]
Patch for landing

Clearing flags on attachment: 119326

Committed r102877: <http://trac.webkit.org/changeset/102877>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.