[webkit-reviews] review granted: [Bug 25415] [GTK][ATK] Please implement support for get_text_at_offset : [Attachment 31051] uselayout.patch

[bugzilla-daemon at webkit.org]

Jan Alonzo <jmalonzo at gmail.com> has granted Xan Lopez <xan.lopez at gmail.com>'s
request for review:
Bug 25415: [GTK][ATK] Please implement support for get_text_at_offset
https://bugs.webkit.org/show_bug.cgi?id=25415

Attachment 31051: uselayout.patch
https://bugs.webkit.org/attachment.cgi?id=31051&action=review

------- Additional Comments from Jan Alonzo <jmalonzo at gmail.com>
> From f00e7bd148421009d2178db814c4ed4cb20c1074 Mon Sep 17 00:00:00 2001
> From: Xan Lopez <xlopez at igalia.com>
> Date: Mon, 8 Jun 2009 18:20:51 +0300
> Subject: [PATCH] 2009-06-08  Xan Lopez  <xlopez at igalia.com>
> 
>	  Reviewed by NOBODY (OOPS!).
> 
>	  https://bugs.webkit.org/show_bug.cgi?id=25415
>	  [GTK][ATK] Please implement support for get_text_at_offset
> 
>	  Pass a PangoLayout to the GailTextUtil function calls.
> 
>	  It's needed for LINE boundary calls to work correctly.
> 
>	  * accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:
>	  (updateLayout):
>	  (getPangoLayoutForAtk):
>	  (webkit_accessible_text_get_text_after_offset):
>	  (webkit_accessible_text_get_text_at_offset):
> ---
>  WebCore/ChangeLog				      |   17 ++++++++
>  .../gtk/AccessibilityObjectWrapperAtk.cpp	      |   41
++++++++++++++++++-
>  2 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 0334758..694f595 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -5,6 +5,23 @@
>	   https://bugs.webkit.org/show_bug.cgi?id=25415
>	   [GTK][ATK] Please implement support for get_text_at_offset
>  
> +	   Pass a PangoLayout to the GailTextUtil function calls.
> +
> +	   It's needed for LINE boundary calls to work correctly.
> +
> +	   * accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:
> +	   (updateLayout):
> +	   (getPangoLayoutForAtk):
> +	   (webkit_accessible_text_get_text_after_offset):
> +	   (webkit_accessible_text_get_text_at_offset):
> +
> +2009-06-08  Xan Lopez  <xlopez at igalia.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=25415
> +	   [GTK][ATK] Please implement support for get_text_at_offset
> +
>	   Use GailUtilText instead of my crappy partial reimplementation of
>	   it. This should add support for LINE boundaries too, although it's
>	   mostly untested for now.
> diff --git a/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
b/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
> index e70aaa0..bddac93 100644
> --- a/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
> +++ b/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
> @@ -41,6 +41,7 @@
>  #include "Editor.h"
>  #include "Frame.h"
>  #include "FrameView.h"
> +#include "HostWindow.h"
>  #include "HTMLNames.h"
>  #include "IntRect.h"
>  #include "NotImplemented.h"
> @@ -503,19 +504,53 @@ static GailTextUtil* getGailTextUtilForAtk(AtkText*
textObject)
>      return gailTextUtil;
>  }
>  
> +// We can use the same callback for both 'style-set' and
> +// 'direction-changed', since we don't care about neither of their
> +// second parameters.
> +static void updateLayout(GtkWidget* widget, gpointer dummy, gpointer
userData)
> +{
> +   gpointer data = g_object_get_data(G_OBJECT(userData),
"webkit-accessible-pango-layout");
> +   if (!data)
> +	  return;
> +
> +   pango_layout_context_changed(static_cast<PangoLayout*>(data));
> +}
> +
> +static PangoLayout* getPangoLayoutForAtk(AtkText* textObject)
> +{
> +    gpointer data = g_object_get_data(G_OBJECT(textObject),
"webkit-accessible-pango-layout");
> +    if (data)
> +	   return static_cast<PangoLayout*>(data);
> +
> +    String text;
> +    AccessibilityObject* coreObject = core(textObject);
> +
> +    if (coreObject->isTextControl())
> +	   text = coreObject->text();
> +    else
> +	   text = coreObject->textUnderElement();
> +
> +    PlatformWidget webView =
coreObject->document()->view()->hostWindow()->platformWindow();

Might be good to null-check hostWindow() and maybe platformWindow() too.

Looks fine. r=me.