Bug 122448

Summary: [ATK] Support compilation of EFL/GTK without accessibility
Product: WebKit Reporter: Krzysztof Czech <k.czech>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, commit-queue, gyuyoung.kim, mario, rakuco, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
cfleizach: review-
patch none

Description Krzysztof Czech 2013-10-07 06:58:21 PDT
I'm proposing a patch that adds missing HAVE(ACCESSIBILITY) guards to ATK's related files so that in case of turning off the HAVE_ACCESSIBILITY macro EFL/GTK would compile properly.
Comment 1 Radar WebKit Bug Importer 2013-10-07 06:58:30 PDT
<rdar://problem/15164815>
Comment 2 Krzysztof Czech 2013-10-07 07:22:22 PDT
Created attachment 213580 [details]
Patch
Comment 3 Krzysztof Czech 2013-10-07 07:36:39 PDT
Created attachment 213581 [details]
Patch
Comment 4 chris fleizach 2013-10-07 12:11:49 PDT
Comment on attachment 213581 [details]
Patch

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

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:1434
> +namespace WTR {

Is there code like this already inside AccessibilityUIElement.cpp?
Comment 5 Krzysztof Czech 2013-10-08 01:30:21 PDT
> (From update of attachment 213581 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=213581&action=review
> 
> > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:1434
> > +namespace WTR {
> 
> Is there code like this already inside AccessibilityUIElement.cpp?
Yes there is, you mean it could be reused ?.
Sounds good, I will check this.
Comment 6 Krzysztof Czech 2013-10-08 03:14:19 PDT
Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:1434
> > > +namespace WTR {
> > 
> > Is there code like this already inside AccessibilityUIElement.cpp?
> Yes there is, you mean it could be reused ?.
> Sounds good, I will check this.

It could be reused as well as I guess bits from AccessibilityUIController.cpp
Line #if !PLATFORM(MAC) && !PLATFORM(GTK) && !PLATFORM(EFL) in AccessibilityUIElement.cpp
can be changed to 
#if (!PLATFORM(MAC) && !PLATFORM(GTK) && !PLATFORM(EFL)) || !HAVE(ACCESSIBILITY)

but then respective mac files should have #if HAVE(ACCESSIBILITY) added, otherwise when accessibility is disabled, redefinition conflicts will happen.
Comment 7 chris fleizach 2013-10-09 14:59:25 PDT
(In reply to comment #6)
> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:1434
> > > > +namespace WTR {
> > > 
> > > Is there code like this already inside AccessibilityUIElement.cpp?
> > Yes there is, you mean it could be reused ?.
> > Sounds good, I will check this.
> 
> It could be reused as well as I guess bits from AccessibilityUIController.cpp
> Line #if !PLATFORM(MAC) && !PLATFORM(GTK) && !PLATFORM(EFL) in AccessibilityUIElement.cpp
> can be changed to 
> #if (!PLATFORM(MAC) && !PLATFORM(GTK) && !PLATFORM(EFL)) || !HAVE(ACCESSIBILITY)
> 
> but then respective mac files should have #if HAVE(ACCESSIBILITY) added, otherwise when accessibility is disabled, redefinition conflicts will happen.

I think we should re-use these. I don't want to have to maintain two long lists of unsupported functions when adding to DRT
Comment 8 Krzysztof Czech 2013-11-12 05:19:36 PST
Created attachment 216655 [details]
patch
Comment 9 Krzysztof Czech 2013-11-12 05:27:30 PST
(In reply to comment #7)
> (In reply to comment #6)
> > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:1434
> > > > > +namespace WTR {
> > > > 
> > > > Is there code like this already inside AccessibilityUIElement.cpp?
> > > Yes there is, you mean it could be reused ?.
> > > Sounds good, I will check this.
> > 
> > It could be reused as well as I guess bits from AccessibilityUIController.cpp
> > Line #if !PLATFORM(MAC) && !PLATFORM(GTK) && !PLATFORM(EFL) in AccessibilityUIElement.cpp
> > can be changed to 
> > #if (!PLATFORM(MAC) && !PLATFORM(GTK) && !PLATFORM(EFL)) || !HAVE(ACCESSIBILITY)
> > 
> > but then respective mac files should have #if HAVE(ACCESSIBILITY) added, otherwise when accessibility is disabled, redefinition conflicts will happen.
> 
> I think we should re-use these. I don't want to have to maintain two long lists of unsupported functions when adding to DRT

Yes, you are right in terms of maintenance, this would not be a good idea to support it. Thanks for the suggestion. I already prepared a new patch.
Comment 10 WebKit Commit Bot 2013-11-13 01:59:51 PST
Comment on attachment 216655 [details]
patch

Clearing flags on attachment: 216655

Committed r159181: <http://trac.webkit.org/changeset/159181>
Comment 11 WebKit Commit Bot 2013-11-13 01:59:54 PST
All reviewed patches have been landed.  Closing bug.