[webkit-reviews] review canceled: [Bug 57854] [GTK] Implement AccessibilityUIElement::{row|column}IndexRange in DRT : [Attachment 88250] Patch proposal + unskipped layout test

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <msanchez at igalia.com> has canceled Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 57854: [GTK] Implement AccessibilityUIElement::{row|column}IndexRange in
DRT
https://bugs.webkit.org/show_bug.cgi?id=57854

Attachment 88250: Patch proposal + unskipped layout test
https://bugs.webkit.org/attachment.cgi?id=88250&action=review

------- Additional Comments from Mario Sanchez Prada <msanchez at igalia.com>
(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').