Bug 21758 - A little more PLATFORM(CHROMIUM) in WebCore
Summary: A little more PLATFORM(CHROMIUM) in WebCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Fisher (:fishd, Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-20 13:02 PDT by Darin Fisher (:fishd, Google)
Modified: 2008-10-22 00:57 PDT (History)
1 user (show)

See Also:


Attachments
v1 patch (1.73 KB, patch)
2008-10-20 13:04 PDT, Darin Fisher (:fishd, Google)
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2008-10-20 13:02:41 PDT
A little more PLATFORM(CHROMIUM) in WebCore

This change covers:
  platform/graphics/Icon.h
  page/AccessibilityObject.h
Comment 1 Darin Fisher (:fishd, Google) 2008-10-20 13:04:10 PDT
Created attachment 24529 [details]
v1 patch

Simple PLATFORM(CHROMIUM) change.
Comment 2 Alp Toker 2008-10-20 16:30:14 PDT
Comment on attachment 24529 [details]
v1 patch

>Index: ChangeLog
>===================================================================
>--- ChangeLog	(revision 37744)
>+++ ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2008-10-20  Darin Fisher  <darin@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        A little more PLATFORM(CHROMIUM) in WebCore
>+        https://bugs.webkit.org/show_bug.cgi?id=21758
>+
>+        * page/AccessibilityObject.h:
>+        * platform/graphics/Icon.h:
>+
> 2008-10-20  Alp Toker  <alp@nuanti.com>
> 
>         Fix autotools dist build target by listing recently added header
>Index: page/AccessibilityObject.h
>===================================================================
>--- page/AccessibilityObject.h	(revision 37733)
>+++ page/AccessibilityObject.h	(working copy)
>@@ -40,6 +40,8 @@
> #elif PLATFORM(WIN)
> #include "AccessibilityObjectWrapperWin.h"
> #include "COMPtr.h"
>+#elif PLATFORM(CHROMIUM)
>+#include "AccessibilityObjectWrapper.h"
> #endif
> 
> typedef struct _NSRange NSRange;
>@@ -407,6 +409,8 @@ protected:
>     COMPtr<AccessibilityObjectWrapper> m_wrapper;
> #elif PLATFORM(GTK)
>     AtkObject* m_wrapper;
>+#elif PLATFORM(CHROMIUM)
>+    RefPtr<AccessibilityObjectWrapper> m_wrapper;
> #endif
> };
> 

It's difficult to review this patch without context. Wouldn't PLATFORM(CHROMIUM) re-use one of the existing accessibility implementations depending on the platform (MSAA, ATK etc.)? If this patch is simply a build fix, you may be better served by looking for a way to disable accessibility in your port for now, for example using ENABLE(ACCESSIBILITY). Not marking r- but this one needs some clarification from the submitter.
Comment 3 Darin Fisher (:fishd, Google) 2008-10-20 17:03:18 PDT
Hi Alp.  Thanks for the great question.  It turns out that using traditional a11y APIs directly from the sandboxed chromium renderer simply doesn't work.  We have to marshal everything out of the sandbox.  This is best accomplished using platform independent code, so for us the AccessibilityObjectWrapper really shouldn't be COM or ATK or any other platform specific technology.  Does that make sense?
Comment 4 Eric Seidel (no email) 2008-10-21 14:53:51 PDT
Comment on attachment 24529 [details]
v1 patch

Looks sane enough.
Comment 5 Darin Fisher (:fishd, Google) 2008-10-22 00:57:33 PDT
http://trac.webkit.org/changeset/37776