[webkit-reviews] review denied: [Bug 25528] [Gtk] Text attributes not exposed : [Attachment 49407] webkit_accessible_text_get_default_attributes and webkit_accessible_text_get_run_attributes implementation
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu May 13 01:06:56 PDT 2010
Xan Lopez <xan.lopez at gmail.com> has denied José Millán Soto
<jmillan at igalia.com>'s request for review:
Bug 25528: [Gtk] Text attributes not exposed
https://bugs.webkit.org/show_bug.cgi?id=25528
Attachment 49407: webkit_accessible_text_get_default_attributes and
webkit_accessible_text_get_run_attributes implementation
https://bugs.webkit.org/attachment.cgi?id=49407&action=review
------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
> Index: WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
> ===================================================================
> --- WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
(revision 55194)
> +++ WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
(working copy)
> @@ -1012,16 +1012,263 @@ static gint webkit_accessible_text_get_c
> return offset;
> }
>
> -static AtkAttributeSet* webkit_accessible_text_get_run_attributes(AtkText*
text, gint offset, gint* start_offset, gint* end_offset)
> +static AtkAttributeSet* getAttributeSetForAccessibilityObject(const
AccessibilityObject* object)
> {
> - notImplemented();
> - return 0;
> + RenderObject* renderer;
> + AtkAttributeSet* result = 0;
> + RenderStyle* style;
> + const int bufferSize = 16;
> + gchar buffer[bufferSize];
> +
> + if (!object->isAccessibilityRenderObject())
> + return 0;
> +
> + renderer = (static_cast<const
AccessibilityRenderObject*>(object))->renderer();
> + style = renderer->style();
> +
> + g_snprintf(buffer, bufferSize, "%i", style->fontSize());
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_SIZE), buffer);
> +
> + if (style->hasBackground()
> + && style->backgroundColor().isValid()) {
> + g_snprintf(buffer, bufferSize, "%i,%i,%i",
style->backgroundColor().red(), style->backgroundColor().green(),
style->backgroundColor().blue());
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_BG_COLOR), buffer);
> + }
> + if (style->color().isValid()) {
> + g_snprintf(buffer, bufferSize, "%i,%i,%i", style->color().red(),
style->color().green(), style->color().blue());
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_FG_COLOR), buffer);
> + }
> +
> + int baselinePosition = renderer->baselinePosition(true);
> + bool includeRise;
> + switch (style->verticalAlign()) {
> + case SUB:
> + baselinePosition = -baselinePosition;
> + case SUPER:
> + includeRise = true;
> + break;
> + case BASELINE:
> + includeRise = true;
> + baselinePosition = 0;
> + break;
> + default:
> + includeRise = false;
> + }
> + if (includeRise) {
> + g_snprintf(buffer, bufferSize, "%i", baselinePosition);
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_RISE), buffer);
> + }
> +
> + int indentation = style->textIndent().calcValue(object->size().width());
> + if (indentation != undefinedLength) {
> + g_snprintf(buffer, bufferSize, "%i", indentation);
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_INDENT), buffer);
> + }
> +
> + String fontFamilyName = style->font().family().family().string();
> + if (fontFamilyName.left(8) == "-webkit-")
> + fontFamilyName = fontFamilyName.substring(8);
> +
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_FAMILY_NAME),
fontFamilyName.utf8().data());
> +
> + int fontWeight = -1;
> + switch (style->font().weight()) {
> + case FontWeight100:
> + fontWeight = 100;
> + break;
> + case FontWeight200:
> + fontWeight = 200;
> + break;
> + case FontWeight300:
> + fontWeight = 300;
> + break;
> + case FontWeight400:
> + fontWeight = 400;
> + break;
> + case FontWeight500:
> + fontWeight = 500;
> + break;
> + case FontWeight600:
> + fontWeight = 600;
> + break;
> + case FontWeight700:
> + fontWeight = 700;
> + break;
> + case FontWeight800:
> + fontWeight = 800;
> + break;
> + case FontWeight900:
> + fontWeight = 900;
> + }
> + if (fontWeight > 0) {
> + g_snprintf(buffer, bufferSize, "%i", fontWeight);
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_WEIGHT), buffer);
> + }
> +
> + switch (style->textAlign()) {
> + case TAAUTO:
> + break;
> + case LEFT:
> + case WEBKIT_LEFT:
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_JUSTIFICATION), "left");
> + break;
> + case RIGHT:
> + case WEBKIT_RIGHT:
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_JUSTIFICATION), "right");
> + break;
> + case CENTER:
> + case WEBKIT_CENTER:
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_JUSTIFICATION), "center");
> + break;
> + case JUSTIFY:
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_JUSTIFICATION), "fill");
> + }
> +
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_UNDERLINE), ((style->textDecoration()
& UNDERLINE) ? "single" : "none"));
> +
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_STYLE), (style->font().italic() ?
"italic" : "normal"));
> +
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_STRIKETHROUGH),
((style->textDecoration() & LINE_THROUGH) ? "true" : "false"));
> +
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_INVISIBLE), ((style->visibility() ==
HIDDEN) ? "true" : "false"));
> +
> + result = addAttributeToSet(result,
atk_text_attribute_get_name(ATK_TEXT_ATTR_EDITABLE), object->isReadOnly() ?
"false" : "true");
> +
> + return result;
> +}
> +
> +static gint compareAttribute(const AtkAttribute* a, const AtkAttribute* b)
> +{
> + return (g_strcmp0(a->name, b->name) || g_strcmp0(a->value, b->value));
> +}
> +
> +// Returns an AtkAttributeSet with the elements of a1 which are either not
present or different in a2.
> +// Neither a1 nor a2 should be used after calling this function.
> +static AtkAttributeSet* attributeSetDifference(AtkAttributeSet* a1,
AtkAttributeSet* a2)
> +{
> + if (!a2)
> + return a1;
> +
> + AtkAttributeSet* i = a1;
> + AtkAttributeSet* found;
> + AtkAttributeSet* toDelete = 0;
> +
> + while (i) {
> + found = g_slist_find_custom(a2, i->data,
(GCompareFunc)compareAttribute);
> + if (found) {
> + AtkAttributeSet* t = i->next;
> + toDelete = g_slist_prepend(toDelete, i->data);
> + a1 = g_slist_delete_link(a1, i);
> + i = t;
> + } else
> + i = i->next;
> + }
> +
> + atk_attribute_set_free(a2);
> + atk_attribute_set_free(toDelete);
> + return a1;
> +}
> +
> +static guint accessibilityObjectLength(const AccessibilityObject* object)
> +{
> + int value = object->stringValue().length();
> + if (value)
> + return value;
> + if (object->isAccessibilityRenderObject()) {
> + const RenderObject* renderer = (static_cast<const
AccessibilityRenderObject*>(object))->renderer();
> + if (renderer->isBR())
> + return 1;
> + }
> + return object->textUnderElement().length();
> +}
> +
> +static const AccessibilityObject* getAccessibilityObjectForOffset(const
AccessibilityObject* object, guint offset, gint* startOffset, gint* endOffset)
> +{
> + const AccessibilityObject* result;
> + guint length = accessibilityObjectLength(object);
> + if (length > offset) {
> + *startOffset = 0;
> + *endOffset = length;
> + result = object;
> + } else {
> + *startOffset = -1;
> + *endOffset = -1;
> + result = 0;
> + }
> +
> + if (object->firstChild()) {
> + AccessibilityObject* child = object->firstChild();
> + guint childPosition = 0;
> + guint childLength;
> + while (child) {
> + childLength = accessibilityObjectLength(child);
> + if ((childLength + childPosition) > offset) {
> + gint childStartOffset;
> + gint childEndOffset;
> + const AccessibilityObject* grandChild =
getAccessibilityObjectForOffset(child, offset-childPosition,
&childStartOffset, &childEndOffset);
> + if (childStartOffset >= 0) {
> + *startOffset = childStartOffset + childPosition;
> + *endOffset = childEndOffset + childPosition;
> + result = grandChild;
> + }
> + break;
> + }
> + childPosition += childLength;
> + child = child->nextSibling();
> + }
> + }
> + return result;
> +}
> +
> +static AtkAttributeSet* getRunAttributesFromAccesibilityObject(const
AccessibilityObject* element, gint offset, gint* startOffset, gint* endOffset)
> +{
> + const AccessibilityObject *child =
getAccessibilityObjectForOffset(element, offset, startOffset, endOffset);
> + if (!child) {
> + *startOffset = -1;
> + *endOffset = -1;
> + return 0;
> + }
> +
> + AtkAttributeSet* defaultAttributes =
getAttributeSetForAccessibilityObject(element);
> + AtkAttributeSet* childAttributes =
getAttributeSetForAccessibilityObject(child);
> +
> + return attributeSetDifference(childAttributes, defaultAttributes);
> +}
> +
> +static AtkAttributeSet* webkit_accessible_text_get_run_attributes(AtkText*
text, gint offset, gint* startOffset, gint* endOffset)
> +{
> + AccessibilityObject* coreObject = core(text);
> + AtkAttributeSet* result;
> +
> + if (!coreObject) {
> + *startOffset = 0;
> + *endOffset = atk_text_get_character_count(text);
> + return 0;
> + }
> +
> + if (offset == -1)
> + offset = atk_text_get_caret_offset(text);
> +
> + result = getRunAttributesFromAccesibilityObject(coreObject, offset,
startOffset, endOffset);
> +
> + if (*startOffset < 0) {
> + *startOffset = offset;
> + *endOffset = offset;
> + }
> +
> + return result;
> }
>
> static AtkAttributeSet*
webkit_accessible_text_get_default_attributes(AtkText* text)
> {
> - notImplemented();
> - return 0;
> + AccessibilityObject* coreObject = core(text);
> + if (!coreObject || !coreObject->isAccessibilityRenderObject())
> + return 0;
> +
> + AtkAttributeSet* result = 0;
> +
> + result = getAttributeSetForAccessibilityObject(coreObject);
> + return result;
> }
>
> static void webkit_accessible_text_get_character_extents(AtkText* text, gint
offset, gint* x, gint* y, gint* width, gint* height, AtkCoordType coords)
> Index: WebKit/gtk/ChangeLog
> ===================================================================
> --- WebKit/gtk/ChangeLog (revision 55194)
> +++ WebKit/gtk/ChangeLog (working copy)
> @@ -1,3 +1,21 @@
> +2010-02-24 José Millán Soto <jmillan at igalia.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + [Gtk] Text attributes not exposed
> + https://bugs.webkit.org/show_bug.cgi?id=25528
> +
> + Added new tests for accessible text attributes
> +
> + * tests/testatk.c:
> + (testWebkitAtkGetTextInParagraphAndBodyModerate):
> + (compAtkAttribute):
> + (compAtkAttributeName):
> + (atkAttributeSetAttributeHasValue):
> + (atkAttributeSetAreEqual):
> + (testWebkitAtkTextAttributes):
> + (main):
> +
> 2010-02-23 Leandro Pereira <leandro at profusion.mobi>
>
> Reviewed by Gustavo Noronha Silva.
> Index: WebKit/gtk/tests/testatk.c
> ===================================================================
> --- WebKit/gtk/tests/testatk.c (revision 55194)
> +++ WebKit/gtk/tests/testatk.c (working copy)
> @@ -38,6 +38,8 @@ static const char* contentsInParagraphAn
>
> static const char* contentsInParagraphAndBodyModerate = "<html><body><p>This
is a test.</p>Hello world.<br /><font color='#00cc00'>This sentence is
green.</font><br />This one is not.</body></html>";
>
> +static const char* textWithAttributes = "<html><head><style>.st1
{font-family: monospace; color:rgb(120,121,122);} .st2
{text-decoration:underline;
background-color:rgb(80,81,82);}</style></head><body><p style=\"font-size:14;
text-align:right;\">This is the <i>first</i><b> sentence of this
text.</b></p><p class=\"st1\">This sentence should have an style applied <span
class=\"st2\">and this part should have another
one</span>.</p><p>x<sub>1</sub><sup>2</sup>=x<sub>2</sub><sup>3</sup></p><p
style=\"text-align:center;\">This sentence is the <strike>last</strike>
one.</p></body></html>";
> +
> static gboolean bail_out(GMainLoop* loop)
> {
> if (g_main_loop_is_running(loop))
> @@ -409,6 +411,7 @@ static void testWebkitAtkGetTextInParagr
> static void testWebkitAtkGetTextInParagraphAndBodyModerate(void)
> {
> WebKitWebView* webView;
> + WebKitWebFrame* webFrame;
> AtkObject* obj;
> AtkObject* obj1;
> AtkObject* obj2;
> @@ -418,6 +421,7 @@ static void testWebkitAtkGetTextInParagr
>
> webView = WEBKIT_WEB_VIEW(webkit_web_view_new());
> g_object_ref_sink(webView);
> + webFrame = webkit_web_view_get_main_frame(webView);
> GtkAllocation alloc = { 0, 0, 800, 600 };
> gtk_widget_size_allocate(GTK_WIDGET(webView), &alloc);
> webkit_web_view_load_string(webView, contentsInParagraphAndBodyModerate,
NULL, NULL, NULL);
> @@ -450,6 +454,159 @@ static void testWebkitAtkGetTextInParagr
> g_object_unref(webView);
> }
>
> +static gint compAtkAttribute(AtkAttribute *a1, AtkAttribute *a2)
> +{
> + gint strcmpVal;
> + strcmpVal = g_strcmp0(a1->name, a2->name);
> + if (strcmpVal)
> + return strcmpVal;
> +
> + return g_strcmp0(a1->value, a2->value);
> +}
> +
> +static gint compAtkAttributeName(AtkAttribute *a1, AtkAttribute *a2)
> +{
> + return g_strcmp0(a1->name, a2->name);
> +}
> +
> +static gboolean atkAttributeSetAttributeHasValue(AtkAttributeSet *set,
AtkTextAttribute attribute, const gchar *value)
> +{
> + GSList *element;
> + AtkAttribute at;
> + gboolean result;
> + at.name = (gchar *)atk_text_attribute_get_name(attribute);
> + element = g_slist_find_custom(set, &at,
(GCompareFunc)compAtkAttributeName);
> + result = (element && !g_strcmp0(((AtkAttribute*)(element->data))->value,
value));
> + if (!result && element)
> + printf("Expected %s got %s\n", value,
((AtkAttribute*)(element->data))->value);
> + return result;
> +}
> +
> +static gboolean atkAttributeSetAreEqual(AtkAttributeSet *set1,
AtkAttributeSet *set2)
> +{
> + if (!set1)
> + return !set2;
> +
> + set1 = g_slist_sort(set1, (GCompareFunc)compAtkAttribute);
> + set2 = g_slist_sort(set2, (GCompareFunc)compAtkAttribute);
> +
> + while (set1) {
> + if (!set2 || compAtkAttribute(set1->data, set2->data))
> + return FALSE;
> +
> + set1 = set1->next;
> + set2 = set2->next;
> + }
> +
> + return (!set2);
> +}
> +
> +static void testWebkitAtkTextAttributes(void)
> +{
> + WebKitWebView* webView;
> + AtkObject* obj;
> + AtkObject* child;
> + GMainLoop* loop;
> + AtkText* childText;
> + AtkAttributeSet *set1, *set2, *set3, *set4;
> + gint startOffset, endOffset, length;
> +
> + webView = WEBKIT_WEB_VIEW(webkit_web_view_new());
> + g_object_ref_sink(webView);
> + GtkAllocation alloc = { 0, 0, 800, 600 };
> + gtk_widget_size_allocate(GTK_WIDGET(webView), &alloc);
> +
> + webkit_web_view_load_string(webView, textWithAttributes, NULL, NULL,
NULL);
> + loop = g_main_loop_new(NULL, TRUE);
> +
> + g_timeout_add(100, (GSourceFunc)bail_out, loop);
> + g_main_loop_run(loop);
> +
> + obj = gtk_widget_get_accessible(GTK_WIDGET(webView));
> + g_assert(obj);
> +
> + child = atk_object_ref_accessible_child(obj, 0);
> + g_assert(child && ATK_IS_TEXT(child));
> + childText = ATK_TEXT(child);
> + set1 = atk_text_get_run_attributes(childText, 0, &startOffset,
&endOffset);
> + g_assert_cmpint(startOffset, ==, 0);
> + g_assert_cmpint(endOffset, ==, 12);
> + g_assert(atkAttributeSetAreEqual(set1, NULL));
> + set1 = atk_text_get_run_attributes(childText, 15, &startOffset,
&endOffset);
> + g_assert_cmpint(startOffset, ==, 12);
> + g_assert_cmpint(endOffset, ==, 17);
> + g_assert(atkAttributeSetAttributeHasValue(set1, ATK_TEXT_ATTR_STYLE,
"italic"));
> + set2 = atk_text_get_run_attributes(childText, 17, &startOffset,
&endOffset);
> + g_assert_cmpint(startOffset, ==, 17);
> + g_assert_cmpint(endOffset, ==, 40);
> + g_assert(atkAttributeSetAttributeHasValue(set2, ATK_TEXT_ATTR_WEIGHT,
"700"));
> + set3 = atk_text_get_default_attributes(childText);
> + g_assert(atkAttributeSetAttributeHasValue(set3, ATK_TEXT_ATTR_STYLE,
"normal"));
> + g_assert(atkAttributeSetAttributeHasValue(set3,
ATK_TEXT_ATTR_JUSTIFICATION, "right"));
> + g_assert(atkAttributeSetAttributeHasValue(set3, ATK_TEXT_ATTR_SIZE,
"14"));
> + atk_attribute_set_free(set1);
> + atk_attribute_set_free(set2);
> + atk_attribute_set_free(set3);
> +
> + child = atk_object_ref_accessible_child(obj, 1);
> + g_assert(child && ATK_IS_TEXT(child));
> + childText = ATK_TEXT(child);
> + set1 = atk_text_get_default_attributes(childText);
> + g_assert(atkAttributeSetAttributeHasValue(set1,
ATK_TEXT_ATTR_FAMILY_NAME, "monospace"));
> + g_assert(atkAttributeSetAttributeHasValue(set1, ATK_TEXT_ATTR_STYLE,
"normal"));
> + g_assert(atkAttributeSetAttributeHasValue(set1,
ATK_TEXT_ATTR_STRIKETHROUGH, "false"));
> + g_assert(atkAttributeSetAttributeHasValue(set1, ATK_TEXT_ATTR_WEIGHT,
"400"));
> + g_assert(atkAttributeSetAttributeHasValue(set1, ATK_TEXT_ATTR_FG_COLOR,
"120,121,122"));
> + set2 = atk_text_get_run_attributes(childText, 43, &startOffset,
&endOffset);
> + g_assert_cmpint(startOffset, ==, 43);
> + g_assert_cmpint(endOffset, ==, 80);
> + // Checks that default attributes of text are not returned when called
to atk_text_get_run_attributes
> + g_assert(!atkAttributeSetAttributeHasValue(set2, ATK_TEXT_ATTR_FG_COLOR,
"120,121,122"));
> + g_assert(atkAttributeSetAttributeHasValue(set2, ATK_TEXT_ATTR_UNDERLINE,
"single"));
> + g_assert(atkAttributeSetAttributeHasValue(set2, ATK_TEXT_ATTR_BG_COLOR,
"80,81,82"));
> + atk_attribute_set_free(set1);
> + atk_attribute_set_free(set2);
> +
> + child = atk_object_ref_accessible_child(obj, 2);
> + g_assert(child && ATK_IS_TEXT(child));
> + childText = ATK_TEXT(child);
> + set1 = atk_text_get_run_attributes(childText, 0, &startOffset,
&endOffset);
> + set2 = atk_text_get_run_attributes(childText, 3, &startOffset,
&endOffset);
> + g_assert(atkAttributeSetAreEqual(set1, set2));
> + atk_attribute_set_free(set2);
> + set2 = atk_text_get_run_attributes(childText, 1, &startOffset,
&endOffset);
> + set3 = atk_text_get_run_attributes(childText, 5, &startOffset,
&endOffset);
> + g_assert(atkAttributeSetAreEqual(set2, set3));
> + g_assert(!atkAttributeSetAreEqual(set1, set2));
> + atk_attribute_set_free(set3);
> + set3 = atk_text_get_run_attributes(childText, 2, &startOffset,
&endOffset);
> + set4 = atk_text_get_run_attributes(childText, 6, &startOffset,
&endOffset);
> + g_assert(atkAttributeSetAreEqual(set3, set4));
> + g_assert(!atkAttributeSetAreEqual(set1, set3));
> + g_assert(!atkAttributeSetAreEqual(set2, set3));
> + atk_attribute_set_free(set1);
> + atk_attribute_set_free(set2);
> + atk_attribute_set_free(set3);
> + atk_attribute_set_free(set4);
> +
> + child = atk_object_ref_accessible_child(obj, 3);
> + g_assert(child && ATK_IS_TEXT(child));
> + childText = ATK_TEXT(child);
> + set1 = atk_text_get_run_attributes(childText, 24, &startOffset,
&endOffset);
> + g_assert_cmpint(startOffset, ==, 21);
> + g_assert_cmpint(endOffset, ==, 25);
> + g_assert(atkAttributeSetAttributeHasValue(set1,
ATK_TEXT_ATTR_STRIKETHROUGH, "true"));
> + set2 = atk_text_get_run_attributes(childText, 25, &startOffset,
&endOffset);
> + g_assert_cmpint(startOffset, ==, 25);
> + g_assert_cmpint(endOffset, ==, 30);
> + g_assert(atkAttributeSetAreEqual(set2, NULL));
> + set3 = atk_text_get_default_attributes(childText);
> + g_assert(atkAttributeSetAttributeHasValue(set3,
ATK_TEXT_ATTR_JUSTIFICATION, "center"));
> + atk_attribute_set_free(set1);
> + atk_attribute_set_free(set2);
> + atk_attribute_set_free(set3);
> +}
> +
> int main(int argc, char** argv)
> {
> g_thread_init(NULL);
> @@ -463,6 +620,7 @@ int main(int argc, char** argv)
> g_test_add_func("/webkit/atk/get_text_at_offset_text_input",
test_webkit_atk_get_text_at_offset_text_input);
> g_test_add_func("/webkit/atk/getTextInParagraphAndBodySimple",
testWebkitAtkGetTextInParagraphAndBodySimple);
> g_test_add_func("/webkit/atk/getTextInParagraphAndBodyModerate",
testWebkitAtkGetTextInParagraphAndBodyModerate);
> + g_test_add_func("/webkit/atk/testWebkitAtkTextAttributes",
testWebkitAtkTextAttributes);
> return g_test_run ();
> }
>
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1014
+
The ChangeLog for Webcore is missing.
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1022
+
A nit, but the style guide says to declare variables when needed, not at the
beginning of the block.
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1036
+ }
One space here?
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1046
+ baselinePosition = -baselinePosition;
My guess is that you are falling through deliberately here to get a negative
rise, with includeRise assigned to true in the next case. Please don't do this,
makes the code much harder to guess just to save one variable assignment.
Init includeRise to true outside the switch, and put a break here.
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1110
+ break;
Put this at the end together with default.
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1147
+ static AtkAttributeSet* attributeSetDifference(AtkAttributeSet* a1,
AtkAttributeSet* a2)
Extra space at the begining of the comment?
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1184
+
Still believe it makes sense to reuse webkit_accessible_text_get_text here.
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1271
+ return result;
return getAttributeSetForAccessibilityObject(coreObject);
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1215
+ }
Let's keep a variable with the current offset and use that as the while
condition instead of using a break ...
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:1200
+ AccessibilityObject* child = object->firstChild();
Just exit early if there's no firstChild, gets rid of one level of indentation.
More information about the webkit-reviews
mailing list