[Webkit-unassigned] [Bug 57854] [GTK] Implement AccessibilityUIElement::{row|column}IndexRange in DRT

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=57854


Mario Sanchez Prada <msanchez at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #88250|0                           |1
        is obsolete|                            |
  Attachment #88250|review?                     |
               Flag|                            |
  Attachment #88259|                            |review?
               Flag|                            |




--- Comment #4 from Mario Sanchez Prada <msanchez at igalia.com>  2011-04-05 09:51:07 PST ---
Created an attachment (id=88259)
 --> (https://bugs.webkit.org/attachment.cgi?id=88259&action=review)
Patch proposal + unskipped layout test

(In reply to comment #3)
> (From update of attachment 88250 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88250&action=review
> 
> > Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:510
> > +    GOwnPtr<gchar> rangeString(g_strdup_printf("{0, 0}"));
> 
> This can just be g_strdup.

Sure thing.

> > Tools/DumpRenderTree/gtk/AccessibilityUIElementGtk.cpp:551
> > +    return JSStringCreateWithUTF8CString(rangeString.get());
> 
> These two methods are almost exactly identical (only difference seems to be to use column or row in the printf). I think you can refactor it to make both call a common method that does either one depending on a parameter, for instance?

Done. I've to say I don't usually like writing helper functions with a boolean parameter to distinguish between two possible situations (row or column range?), but I guess in this case, being a _private_ helper function only used from there, it's more than enough (defining an enumeration for this would look to me 'overkill').

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.