Bug 89226

Summary: [GTK] Get rid of DumpRenderTreeSupportGtk::{in|de}crementAccessibilityValue
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, mrobinson
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 89223    
Attachments:
Description Flags
Patch proposal mrobinson: review+, mrobinson: commit-queue-

Mario Sanchez Prada
Reported 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
Attachments
Patch proposal (11.04 KB, patch)
2012-06-15 09:58 PDT, Mario Sanchez Prada
mrobinson: review+
mrobinson: commit-queue-
Mario Sanchez Prada
Comment 1 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.
Martin Robinson
Comment 2 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));
Mario Sanchez Prada
Comment 3 2012-06-18 04:17:56 PDT
Note You need to log in before you can comment on or make changes to this bug.