Possibility to turn off accessibility feature for WebKit-EFL.
Created attachment 177339 [details] proposed patch I had this to change in my environment as I needed it to proceed - I decided to share it.
Comment on attachment 177339 [details] proposed patch Rejecting attachment 177339 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ripts/update-webkit line 152. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource From http://git.chromium.org/external/Webkit 9961a7b..ef0ca77 HEAD -> origin/HEAD error: Ref refs/remotes/origin/master is at ef0ca77a554ed2a07b001ee4935266d6301b2bf8 but expected 9961a7b754078ea893b086fbc787c57745c8a4db ! 9961a7b..ef0ca77 master -> origin/master (unable to update local ref) Died at Tools/Scripts/update-webkit line 152. Full output: http://queues.webkit.org/results/15101901
We will not turn off accessibility (ATK) only by removing ATK from building process. There is still ATK dependant code in WebCore. Global HAVE_ACCESSIBILITY macro is always defined for EFL and some specific ATK code is covered by this macro. Turning off Accessibility completely requires some additional changes in WebCore. I started doing this part but there are some things that should be considered.
(In reply to comment #3) > We will not turn off accessibility (ATK) only by removing ATK from building process. There is still ATK dependant code in WebCore. Global HAVE_ACCESSIBILITY macro is always defined for EFL and some specific ATK code is covered by this macro. Turning off Accessibility completely requires some additional changes in WebCore. I started doing this part but there are some things that should be considered. Thanks Krzysztof, I just noticed as well that my patch is not complete. As the patch is not landed yet, I will make it obsolete and please feel free to pick it up from where I left it and make it complete. For my need the most important things is to have a build option to disable the ATK dependent parts (when ATK is not available). It is not a strictly needed to disable all the accessibility bits, but certainly that would be best solution.
Created attachment 178547 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL.
Attachment 178547 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1 Source/WebCore/PlatformEfl.cmake:349: One space between command "endif" and its parentheses, should be "endif (" [whitespace/parentheses] [5] Source/WebCore/accessibility/AccessibilityObject.h:785: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 178547 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. Attachment 178547 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/15230635
Comment on attachment 178547 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. Attachment 178547 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/15231505
Comment on attachment 178547 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. Attachment 178547 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15228646
Created attachment 178552 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL.
Attachment 178552 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'ChangeLog', u'Source/WebCore/ChangeLog', u..." exit_code: 1 Source/WebCore/PlatformEfl.cmake:349: One space between command "endif" and its parentheses, should be "endif (" [whitespace/parentheses] [5] Source/WebCore/accessibility/AccessibilityObject.h:785: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 178553 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL.
I added some additional bits so that we can completely turn off the Accessibility feature for EFL. Modifications in WebCore/PlatformEfl.cmake may not give us desired effect. If we turn off a11y by only disabling it in WebCore/PLatformEfl.cmake, compilation errors appear due to the fact, there are ATK routines left in WebKit/accessibility/. I added additional macro ENABLE_ACCESSIBILITY_EFL so that we can play with those ATK routines.
Created attachment 178562 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL.
Comment on attachment 178553 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. there's already a HAVE(ACCESSIBILITY) macro. can you use that?
Comment on attachment 178562 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. View in context: https://bugs.webkit.org/attachment.cgi?id=178562&action=review > Source/cmake/OptionsEfl.cmake:217 > + add_definitions(-DENABLE_ACCESSIBILITY_EFL=1) I think ACCESSIBILITY_EFL is not needed. PLATFORM(EFL) && ENABLE(ACCESSIBILITY) is enough. BTW, why should we use them together ? Can't we use PLATFORM(EFL) as GTK port ?
Comment on attachment 178562 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. View in context: https://bugs.webkit.org/attachment.cgi?id=178562&action=review > Source/WebCore/accessibility/AXObjectCache.h:273 > +#if (PLATFORM(EFL) && !ENABLE(ACCESSIBILITY_EFL)) Would it be possible to simply use "#if !HAVE(ACCESSIBILITY)" here and combine this list with the list above" ? > Source/cmake/OptionsEfl.cmake:51 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_ACCESSIBILITY ON) > WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_3D_RENDERING ON) > WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_ANIMATION_API ON) It would be good to keep this list sorted. >> Source/cmake/OptionsEfl.cmake:217 >> + add_definitions(-DENABLE_ACCESSIBILITY_EFL=1) > > I think ACCESSIBILITY_EFL is not needed. PLATFORM(EFL) && ENABLE(ACCESSIBILITY) is enough. BTW, why should we use them together ? Can't we use PLATFORM(EFL) as GTK port ? I think this should be changed to add_definitions(-DHAVE_ACCESSIBILITY=1)
Created attachment 178768 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL.
(In reply to comment #17) > (From update of attachment 178562 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178562&action=review > > > Source/WebCore/accessibility/AXObjectCache.h:273 > > +#if (PLATFORM(EFL) && !ENABLE(ACCESSIBILITY_EFL)) > > Would it be possible to simply use "#if !HAVE(ACCESSIBILITY)" here and combine this list with the list above" ? > > > Source/cmake/OptionsEfl.cmake:51 > > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_ACCESSIBILITY ON) > > WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_3D_RENDERING ON) > > WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_ANIMATION_API ON) > > It would be good to keep this list sorted. > > >> Source/cmake/OptionsEfl.cmake:217 > >> + add_definitions(-DENABLE_ACCESSIBILITY_EFL=1) > > > > I think ACCESSIBILITY_EFL is not needed. PLATFORM(EFL) && ENABLE(ACCESSIBILITY) is enough. BTW, why should we use them together ? Can't we use PLATFORM(EFL) as GTK port ? > > I think this should be changed to add_definitions(-DHAVE_ACCESSIBILITY=1) You are right Laszlo, add_definitions(-DHAVE_ACCESSIBILITY=1) it's a good idea. Having this in OptionsEfl.cmake give us possibility to switch on/off WebKit-EFL accessibility. I moved its declaration for EFL platform to OptionsEfl.cmake.
(In reply to comment #16) > (From update of attachment 178562 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178562&action=review > > > Source/cmake/OptionsEfl.cmake:217 > > + add_definitions(-DENABLE_ACCESSIBILITY_EFL=1) > > I think ACCESSIBILITY_EFL is not needed. PLATFORM(EFL) && ENABLE(ACCESSIBILITY) is enough. BTW, why should we use them together ? Can't we use PLATFORM(EFL) as GTK port ? Yes I agree, ACCESSIBILITY_EFL is not needed. Laszlo pointed out that HAVE_ACCESSIBILITY can be used instead. > BTW, why should we use them together ? Can't we use PLATFORM(EFL) as GTK port ? In some of the places PLATFORM(EFL) is not enough since ATK library is external lib and in sources there are some ATK routines. That's way we got to have second guard.
(In reply to comment #15) > (From update of attachment 178553 [details]) > there's already a HAVE(ACCESSIBILITY) macro. can you use that? Yes I can use this macro. I corrected my patch.
Comment on attachment 178768 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. View in context: https://bugs.webkit.org/attachment.cgi?id=178768&action=review Overall looks good to me, but it would be good to fix the remaining minor issues. > Source/WTF/wtf/Platform.h:674 > -#if PLATFORM(IOS) || PLATFORM(MAC) || PLATFORM(WIN) || PLATFORM(GTK) || (PLATFORM(CHROMIUM) && !OS(ANDROID)) || PLATFORM(EFL) > +#if PLATFORM(IOS) || PLATFORM(MAC) || PLATFORM(WIN) || PLATFORM(GTK) || (PLATFORM(CHROMIUM) && !OS(ANDROID)) I prefer to keep PLATFORM(EFL) listed here as that is still the default for EFL. We can still disable ACCESSIBILITY (see the comment below). If you remove this change than you do not need to change WTF. > Source/WebCore/accessibility/AccessibilityObject.h:784 > -#elif !PLATFORM(CHROMIUM) > +#elif !PLATFORM(CHROMIUM) Extra white space. This change is not needed. > Source/cmake/OptionsEfl.cmake:218 > +if (ENABLE_ACCESSIBILITY) > + find_package(ATK REQUIRED) > + add_definitions(-DHAVE_ACCESSIBILITY=1) > +endif () I think it is better to have the default in .h file and have the non-typical option (the ability to disable) in the make system. What do you think of the following: if (ENABLE_ACCESSIBILITY) find_package(ATK REQUIRED) else () add_definitions(-DHAVE_ACCESSIBILITY=0) endif ()
Created attachment 179052 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL.
Created attachment 179058 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL.
Comment on attachment 179058 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. lgtm.
Comment on attachment 179058 [details] [EFL] Possibility to turn off accessibility feature for WebKit-EFL. Clearing flags on attachment: 179058 Committed r137475: <http://trac.webkit.org/changeset/137475>
All reviewed patches have been landed. Closing bug.