[Webkit-unassigned] [Bug 197413] [WPE] Add initial accessibility support using ATK

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=197413

--- Comment #7 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #5)
> Comment on attachment 370043 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370043&action=review
> 
> > Source/WebKit/PlatformWPE.cmake:291
> >  list(APPEND WebKit_LIBRARIES
> >      PRIVATE
> > +        ${ATK_LIBRARIES}
> 
> Heh, there's that PRIVATE again... I know this is a preexisting problem, but
> that's broken.

I have no idea what this is for, I just copy-paste when I have to do CMake stuff. So, tell me what I should here, just remove it?

> > Source/WebKit/UIProcess/API/wpe/WPEView.h:63
> > +    ~View();
> 
> Should we make this class noncopyable now? Probably?

Why now?

> > Source/WebKit/UIProcess/API/wpe/WPEView.h:87
> > +    WebKitWebViewAccessible* accessible();
> 
> I would make it const, so it can be used in const functions.
> 
> > Source/WebKit/UIProcess/API/wpe/WPEView.h:110
> > +    GRefPtr<WebKitWebViewAccessible> m_accessible;
> 
> And make it mutable, so it can be set in WPEView::accessible once that
> becomes const.
> 
> > Source/WebKit/WebProcess/WebPage/wpe/WebPageWPE.cpp:43
> > +    // entry point to the Web process, and send a message to the UI
> 
> web process
> 
> > Source/WebKit/WebProcess/wpe/WebProcessMainWPE.cpp:69
> > +    atkUtilClass->get_toolkit_version = []() -> const gchar* {
> > +        return "";
> > +    };
> 
> You could g_strdup_printf("%d.%d.%d", wpe_get_major_version(),
> wpe_get_minor_version(), wpe_get_micro_version()), store it in a CString
> cstr, and return cstr.data()?

This is in the web process, we could just expose versions numbers to the build instead. I know we could still use it, but it's kind of a layering violation IMO.

> > Tools/WebKitTestRunner/InjectedBundle/AccessibilityController.h:76
> > -#elif PLATFORM(GTK) || PLATFORM(EFL)
> > +#elif USE(ATK)
> 
> Can't believe we missed this... I did a git grep to make sure there are no
> more places. Seems you get the credit for completely removing EFL in the end.
> 
> > Tools/wpe/backends/ViewBackend.cpp:232
> > +    atkUtilClass->get_toolkit_version = []() -> const gchar* {
> > +        return "";
> > +    };
> 
> Ditto regarding version.

Tooling backends don't use the public api either. We don't even have WTF there.

> > Tools/wpe/backends/WebKitAccessibleApplication.cpp:88
> > +    g_type_class_add_private(klass, sizeof(WebKitAccessibleApplicationPrivate));
> 
> You know this is deprecated? Modern GObjects should use G_ADD_PRIVATE() in
> the G_DEFINE_TYPE() up above, then use
> webkit_accessible_application_get_instance_private() to get the priv struct.
> Anyway, your choice.

Yes, I know it, but I think we still depend on a previous version.

> > Tools/wpe/jhbuild.modules:292
> > +  <meson id="atk" mesonargs="-Dintrospection=false">
> > +    <branch module="pub/GNOME/sources/atk/2.32/atk-2.32.0.tar.xz" version="2.32.0"
> > +            repo="ftp.gnome.org"
> > +            hash="sha256:cb41feda7fe4ef0daa024471438ea0219592baf7c291347e5a858bb64e4091cc"/>
> > +    <dependencies>
> > +      <dep package="glib"/>
> > +    </dependencies>
> > +  </meson>
> > +
> > +  <meson id="at-spi2-core" mesonargs="-Dintrospection=no">
> 
> Seriously, one uses -Dintrospection=false and the other -Dintrospection=no?
> :S

Yes :-/

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190517/6f8feea7/attachment.html>