Bug 89226 - [GTK] Get rid of DumpRenderTreeSupportGtk::{in|de}crementAccessibilityValue
Summary: [GTK] Get rid of DumpRenderTreeSupportGtk::{in|de}crementAccessibilityValue
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 89223
  Show dependency treegraph
 
Reported: 2012-06-15 09:34 PDT by Mario Sanchez Prada
Modified: 2012-06-18 04:17 PDT (History)
2 users (show)

See Also:


Attachments
Patch proposal (11.04 KB, patch)
2012-06-15 09:58 PDT, Mario Sanchez Prada
mrobinson: review+
mrobinson: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2012-06-15 09:34:36 PDT
Currentĺy, in order to unskip tests like accessibility/input-slider.html in WKGTK, we have this helper functions in DumpRenderTreeSupportGtk, since there's no equivalent function available in ATK API.

However, it should be possible to implement them in DRT without needing this helper functions, just by using functions defined in the AtkValue interface:

   atk_value_get_current_value()
   atk_value_get_minimum_increment()
   atk_value_set_current_value()

Besides allowing to remove unnecesary code from DumpRenderTreeSupportGtk, this change will be useful too for implementing the same functions in WebKitTestRunner, where we will have the same situation. Thus, blocking bug 89223 for this reason
Comment 1 Mario Sanchez Prada 2012-06-15 09:58:27 PDT
Created attachment 147841 [details]
Patch proposal

No layout test needed, as the test for this is that tests like accessibility/input-slider.html keep working, as well as that tests in WK2 (when enabled) will run properly too.
Comment 2 Martin Robinson 2012-06-15 10:52:45 PDT
Comment on attachment 147841 [details]
Patch proposal

View in context: https://bugs.webkit.org/attachment.cgi?id=147841&action=review

> Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceValue.cpp:78
> +    double minValue = coreObject->minValueForRange();
> +    newValue = newValue < minValue ? minValue : newValue;
> +
> +    double maxValue = coreObject->maxValueForRange();
> +    newValue = newValue > maxValue ? maxValue : newValue;

This can simply be: 
newValue = std::max(coreObject->minValueForRange(), newValue);
newValue = std::min(coreObject->maxValueForRange(), newValue);

> Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:649
> +static void alterCurrentValue(PlatformUIElement element, bool isIncrement)

Instead of a boolean it would make sense to either pass a sign value ala alterCurrentValue(element, -1) and  alterCurrentValue(element, 1) or create an enum AlterCurrentValueOperation { Increment, Decrement };

The nice thing about the sign approach is that below you can just write:

double newDoubleValue = g_value_get_double(&currentValue) + (sign * g_value_get_double(&increment));
Comment 3 Mario Sanchez Prada 2012-06-18 04:17:56 PDT
Committed r120588: <http://trac.webkit.org/changeset/120588>