Bug 111986 - [EFL] accessibility/aria-invalid is failing
Summary: [EFL] accessibility/aria-invalid is failing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 111985
  Show dependency tree / graph
 
Reported: 2013-03-11 06:54 PDT by Krzysztof Czech
Modified: 2013-09-23 08:46 PDT (History)
4 users (show)

See Also:

Attachments
Patch (6.58 KB, patch)
2013-09-23 03:41 PDT, Krzysztof Czech 		mario: review-
Details | Formatted Diff | Diff
Patch (6.44 KB, patch)
2013-09-23 06:36 PDT, Krzysztof Czech 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Krzysztof Czech 2013-03-11 06:54:51 PDT 
accessibility/aria-invalid is failing is failing on all EFL platforms.
Comment 1 Denis Nomiyama (dnomi) 2013-09-20 09:23:24 PDT 
The implementation of aria-invalid has been added to the GTK port in bug 98350 and bug 121668.
The EFL port can also use this code by implementing a small piece of code at:

Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp
- axObjectEventListener()
- addAccessibilityNotificationHandler()
- removeAccessibilityNotificationHandler()

and

Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp
- axObjectEventListener()
- setNotificationFunctionCallback()
- removeAccessibilityNotificationHandler()

Currently there is only implementation for the GTK platform (e.g. #if PLATFORM(GTK)), and the code needs basically to get the JS context.
Comment 2 Krzysztof Czech 2013-09-23 01:27:59 PDT 
(In reply to comment #1)
> The implementation of aria-invalid has been added to the GTK port in bug 98350 and bug 121668.
> The EFL port can also use this code by implementing a small piece of code at:
> 

Great, I will take a look at this. Thanks
Comment 3 Krzysztof Czech 2013-09-23 03:41:50 PDT 
Created attachment 212339 [details]
Patch
Comment 4 Mario Sanchez Prada 2013-09-23 03:59:28 PDT 
Comment on attachment 212339 [details]
Patch

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

> Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp:131
> +#elif PLATFORM(EFL) && HAVE(ACCESSIBILITY)

The whole content of this file is wrapper inside an HAVE(ACCESSIBILITY) guard, so you don't need to add it explicitly here.

> Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp:232
> +#elif PLATFORM(EFL) && HAVE(ACCESSIBILITY)

Same here

> Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp:266
> +#elif PLATFORM(EFL) && HAVE(ACCESSIBILITY)

And here :)

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp:116
> +#if PLATFORM(GTK) || (PLATFORM(EFL) && HAVE(ACCESSIBILITY))

In this case is different because this file is not guarded at all as the other ones.

Perhaps we should do it here too? After all, this file does not make much sense at all if you don't have accessibility enabled, does it? :)
Comment 5 Krzysztof Czech 2013-09-23 04:13:41 PDT 
 > > Tools/DumpRenderTree/atk/AccessibilityCallbacksAtk.cpp:131
> > +#elif PLATFORM(EFL) && HAVE(ACCESSIBILITY)
> 
> The whole content of this file is wrapper inside an HAVE(ACCESSIBILITY) guard, so you don't need to add it explicitly here.
> 
Indeed, sorry, I did not catch this.
> > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp:116
> > +#if PLATFORM(GTK) || (PLATFORM(EFL) && HAVE(ACCESSIBILITY))
> 
> In this case is different because this file is not guarded at all as the other ones.
> 
> Perhaps we should do it here too? After all, this file does not make much sense at all if you don't have accessibility enabled, does it? :)

Yes, you're right, we should have this guard. Those are specific accessibility bits.
Comment 6 Krzysztof Czech 2013-09-23 06:36:59 PDT 
Created attachment 212344 [details]
Patch
Comment 7 Krzysztof Czech 2013-09-23 06:39:44 PDT 
> In this case is different because this file is not guarded at all as the other ones.
> 
> Perhaps we should do it here too? After all, this file does not make much sense at all if you don't have accessibility enabled, does it? :)

I will propose a new patch with guards
Comment 8 Mario Sanchez Prada 2013-09-23 08:23:02 PDT 
Comment on attachment 212344 [details]
Patch

lgtm
Comment 9 WebKit Commit Bot 2013-09-23 08:46:36 PDT 
Comment on attachment 212344 [details]
Patch

Clearing flags on attachment: 212344

Committed r156273: <http://trac.webkit.org/changeset/156273>
Comment 10 WebKit Commit Bot 2013-09-23 08:46:38 PDT 
All reviewed patches have been landed.  Closing bug.