[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
bugzilla-daemon at webkit.org
Tue Apr 5 09:51:07 PDT 2011
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').
More information about the webkit-reviews
mailing list